Bug 95709 - [CellEditors] [nls tooling] Strange selection behaviour in the NLS table
Summary: [CellEditors] [nls tooling] Strange selection behaviour in the NLS table
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P3 major with 1 vote (vote)
Target Milestone: 3.1.1   Edit
Assignee: JDT-Text-Inbox CLA Friend
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-05-18 03:49 EDT by Martin Aeschlimann CLA Friend
Modified: 2005-08-09 06:17 EDT (History)
2 users (show)

See Also:

Attachments
ExternalizeWizardPage.patch (584 bytes, patch)
2005-07-06 13:51 EDT, Susan McCourt CLA Friend 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Aeschlimann CLA Friend 2005-05-18 03:49:16 EDT 
M7

1. open an already translated file with lots of strings in the NLS wizard, e.g.
UserLibraryPreferencePage
2. select the first value so that the cell ediotor appears
3. use the scroll bar to scoll so that the (still selected) entry is not visible
anymore
4. select a new entry
  -> the table suddendly scrolls and a different entry than intended gets the
selection
Comment 1 Dani Megert CLA Friend 2005-05-18 05:31:03 EDT 
Note, this is not regression introduced by the new Eclipse NLS support but a
problem of the table since the beginning.
Comment 2 Martin Aeschlimann CLA Friend 2005-06-06 09:25:55 EDT 
General cell modfier problem. Raising severity to major, very annoying behaviour.
Comment 3 Susan McCourt CLA Friend 2005-07-06 13:50:22 EDT 
This behavior is caused by the use of TableViewer.refresh(true) in the middle 
of the cell modification callback.  The CellModifier in ExternalizeWizardPage 
calls validateKeys(true), which will refresh the entire table.  This includes 
restoring the selection in the table after it is refreshed, yet the selection 
is not properly maintained at this point in the callback.  I think the best 
way to fix this is to avoid the problem.  There is no need to refresh the 
entire table in this callback.  Instead, only the item that changed should be 
updated.  Attaching proposed patch.
Comment 4 Susan McCourt CLA Friend 2005-07-06 13:51:30 EDT 
Created attachment 24390 [details]
ExternalizeWizardPage.patch

proposed change to ExternalizeWizardPage
Comment 5 Susan McCourt CLA Friend 2005-07-06 13:54:00 EDT 
Tod - a side question.  Why is the CellModifier.modify callback sent even when 
the value of the cell did not change?  Is this intentional?  Should clients be 
checking for a change in value?  If so, then the ExternalizeWizardPage could 
be further optimized so that nothing is done if the value didn't change.
Comment 6 Tod Creasey CLA Friend 2005-07-06 14:10:08 EDT 
I don't think it is intentional. As long as we don't break API (and made the API
more explicit) this should be fine
Comment 7 Susan McCourt CLA Friend 2005-07-06 14:31:56 EDT 
Opened bug #102906 to track the fact that the cell modify callback is 
triggered even when the value did not change.

Note that the proposed patch is still the suggested fix for this bug, since 
the strange selection occurs whenever the modify callback is (legitimately) 
received.
Comment 8 Susan McCourt CLA Friend 2005-07-07 11:50:35 EDT 
moving to JDT-UI to consider patch for the wizard.
Comment 9 Dani Megert CLA Friend 2005-07-07 11:57:10 EDT 
Considering for 3.1.1.
Comment 10 Martin Aeschlimann CLA Friend 2005-07-07 11:59:59 EDT 
verfied and released patch in jdt.ui HEAD (3.2) > 20050707

suggest as 3.1.1 candidate: simple fix that IMO fixes a really nasty usability
problem in the NLS wizard

Thanks Susan for the patch!
Comment 11 Dani Megert CLA Friend 2005-07-07 12:02:19 EDT 
OK. Approving for 3.1.1. Martin, can you backport it for me ;-) Thanks.
Comment 12 Martin Aeschlimann CLA Friend 2005-07-11 04:57:35 EDT 
patch released in 3.1.1 stream
Comment 13 Tom Hofmann CLA Friend 2005-07-11 06:14:23 EDT 
reviewed patch - ok.
Comment 14 Dani Megert CLA Friend 2005-08-09 04:33:21 EDT 
Verifying...
Comment 15 Dani Megert CLA Friend 2005-08-09 06:17:07 EDT 
Verified in M20050804-1200.