Community
Participate
Working Groups
Bug 334746 An right-to-left control needs an image list with right-to-left support (ILC_MIRROR). In the past, we would only create image list with right-to-left for controls that were created with right-to-left. With bug 29779, any control can be changed to right-to-left after it is created, so we started to create all image list with right-to-left support just in case: if a control changes to rtl or ltr the image list never needs to change. The problem is that image lists with ILC_MIRROR are slower (bug 334746). So I had to revert the changes from bug 29779, we need to rework setOrientation() to recreate the image list when the orientation changes. Here are the list of places where ImageList are used: Button#_setImage Button#enableWidget Menu#imageIndex Tabfolder#imageIndex Table#imageIndex Table#imageIndexHeader Toolitem#updateImages
I'm assigning this bug to Helena Halperin since she was the developer of setOrientation() for windows. Please provide a fix ASAP.
Created attachment 192140 [details] sample patch for tabfolder This is the fix for TabFolder, we believe the same pattern can be used to fix Table, Tree, Button, Toolbar (not sure about Menu, apparently image lists are only used in menu for wince).
Created attachment 192142 [details] sample patch for tabfolder SSQ pointed out a problem on the first patch, we can't relie on the size of the first image in the array to init the new ImageList, we need to use the size o the old image list instead.
Created attachment 192186 [details] patch for tabfolder My apologies, the last code has a error, the image list can't be set in the control (TCM_SETIMAGELIST) till the size is set (or till the first image is added). The patch changes ImageList to init the size in the constructor.
I released the last patch so that Helena can use the new code to fix the other controls.
Created attachment 192199 [details] patch for button, tree and table This is patch for Button, Tree and Table. Please review
(In reply to comment #6) > Created attachment 192199 [details] > patch for button, tree and table > > This is patch for Button, Tree and Table. Please review For button, the code that recreate the image list in updateOrienatation and enableWidget are basically the same, you should be able to share that code by refactoring the code in a new method. Table and Tree. Mostly fine, except that in some cases the columns have the index to the image in the column struct (HDITEM#iImage). This index is not being update to the index of the image in the new image list.
Created attachment 192271 [details] a new patch for button, tree, table and toolbar Button - added a new function "updateImageList" for shared code Tree and Table - added code to update HDITEM Toolbar, added in this patch. I used item.updateImages to resolve the problems with setOrientation for disabled toolbar
Table and Tree are wrong. - does not handle the case OS.COMCTL32_MAJOR >= 6 (Table) - You should only set hdItem.iImage when hdItem.fmt == OS.HDF_IMAGE. - int colIndex = indexOf (column); looks wrong, isn't "i" always equals to colIndex in your code ? See the implementation for indexOf (column) - Likewise, "if (colIndex > -1)" can that ever be false ? - Table only, since the items array in Table can be compressed (see bug 129703) no code should be accessing the items in the array directly. Use this code instead: int count = (int)/*64*/OS.SendMessage (handle, OS.LVM_GETITEMCOUNT, 0, 0); for (int i=0; i<count; i++) { TableItem item = _getItem (i, false); if (item != null) { ... ToolBar also looks wrong. - Why do you call item.updateImages (item.isEnabled ()); ? It seems you are doing the work twice. Besides, item.isEnabled() is wrong here. -if (item == null) break; is wrong as you can have null in the middle of the array.
fixed for Button, I changed the patch I bit, no need to save the size of the previous image list cause buttons can only have one image. The code in updateImageList() is exactly the code that was at end of enableWidget().
Created attachment 193438 [details] patch for tree, table and toolbar changes for tree, table and toolbar
Fixed in HEAD Changes for Table, Tree: 1) for (int i = 0; i < count; i++) { TableItem item = _getItem (i, false); if (item != null) image = item.image; if (image != null) { ... } } This code is wrong, the value in the variable image is not reset for each iteration of the loop. I fixed it for items and columns in Table (in Tree you used the right code). 2) I moved the "if (hwndHeader != 0) {" outside of the for statement over the columns array. No point doing anywork if hwndHeader==0. 3) You were adding all column images to the imageList, the image should only be added to the imageList only when (hdItem.fmt & OS.HDF_IMAGE)!= 0. Toolbar, I removed part of the code I think was not necessary (hopefully I didn't break anything).
This version of toolbar code does not work correct for disabled toolbar. To test, disable toolbar and switch orientation between RTL and LTR. Images disappeared. The reason is a desynchronization between imageList and disabledImageList. "Add" function, applied to imageList and disabledImageList, returns different indexes in several cases. This is because of item.disabledImage2.dispose (), that disposes the image in image list, and function "add" puts the image in this place. This is the reason for use "add" or "put" image in different cases. Another issue , in TabFolder, this code looks incorrect for (int i = 0; i < items.length; i++) { TabItem item = items[0]; if (item == null) break; Image image = item.image; ...
(In reply to comment #13) > This version of toolbar code does not work correct for disabled toolbar. To > test, disable toolbar and switch orientation between RTL and LTR. Images > disappeared. The reason is a desynchronization between imageList and > disabledImageList. "Add" function, applied to imageList and disabledImageList, > returns different indexes in several cases. This is because of > item.disabledImage2.dispose (), that disposes the image in image list, and > function "add" puts the image in this place. This is the reason for use "add" > or "put" image in different cases. See Bug 344392 > > Another issue , in TabFolder, this code looks incorrect > for (int i = 0; i < items.length; i++) { > TabItem item = items[0]; > if (item == null) break; > Image image = item.image; > ... I checked the code and that items array can only have null at the end. So I think the code is fine. am I wrong, do you have case that shows a problem ?
(In reply to comment #14) > > Another issue , in TabFolder, this code looks incorrect > > for (int i = 0; i < items.length; i++) { > > TabItem item = items[0]; > > if (item == null) break; > > Image image = item.image; > > ... > I checked the code and that items array can only have null at the end. So I > think the code is fine. > am I wrong, do you have case that shows a problem Sorry, I was not clear Should not it be TabItem item = items[i]; instead of TabItem item = items[0]; ? After "setOrientation", the TabFolder with different images dispalys the image of the first tab in all tabs.
(In reply to comment #15) > Should not it be > TabItem item = items[i]; > instead of > TabItem item = items[0]; > ? Yes, definitely. Thank you for point this out. Released in HEAD.
