Community
Participate
Working Groups
When compiling with GSEAL_ENABLE flag, there are 2 warnings regarding GtkAccellabel that has no member accel_string. Please take a look at this patch: http://fedorapeople.org/cgit/aspektor/public_git/eclipse.platform.swt.git/commit/?h=gtk_accel_string I am not 100% sure that this is the exact way how it should be, but it works. Why I have removed: int /*long*/ oldPtr = OS.GTK_ACCEL_LABEL_GET_ACCEL_STRING (label); I have noticed that when I added function: OS.gtk_accel_label_set_accel_widget (label, handle); instead of deprecated constant, "oldPtr" started producing memory leaks when I run ControlExample and also when I was running this test program: http://www.java2s.com/Tutorial/Java/0280__SWT/SetAcceleratorforMenuItem.htm When I have removed it, issues disappeared. If you still think this line should be there please let me know. Also, I have tried replicating GTK_ACCEL_LABEL_GET_ACCEL_STRING with different functions from GtkAccelLabel and GtkLabel but without any success.
The patch cannot be right, the API gtk_accel_label_set_accel_widget() takes a GtkWidget as the second parameter (not a char *). That is probably why it is leaking. I am not sure there is any way to replace GTK_ACCEL_LABEL_GET_ACCEL_STRING using GTK API. The SWT and GTK APIs are different with respect to the accelerator text that shows up in the menu item (i. e Ctrl+A). SWT expects to set text which comes after the tab character in the menu item text (i.e MenuItem.setText()). GTK builds the accelerator text from the accelerator keycode and modifiers. GTK does not give any API to set the accelerator text directly.
(In reply to comment #1) > The patch cannot be right, the API gtk_accel_label_set_accel_widget() takes > a GtkWidget as the second parameter (not a char *). That is probably why it > is leaking. > I am passing "handle" as the second parameter which is GtkWidget. I am just saying put label on the the "handle" widget. Why it is wrong ? > I am not sure there is any way to replace GTK_ACCEL_LABEL_GET_ACCEL_STRING > using GTK API. The SWT and GTK APIs are different with respect to the > accelerator text that shows up in the menu item (i. e Ctrl+A). SWT expects > to set text which comes after the tab character in the menu item text (i.e > MenuItem.setText()). GTK builds the accelerator text from the accelerator > keycode and modifiers. GTK does not give any API to set the accelerator > text directly. There should be some way around GTK_ACCEL_LABEL_GET_ACCEL_STRING ? All it does is checks if there is something in the memory saved in "accel_string" and clears it.
(In reply to comment #2) > I am passing "handle" as the second parameter which is GtkWidget. I am just > saying put label on the the "handle" widget. Why it is wrong ? Sorry, I misread the code. It is fine. You can achieve the same result by commenting this line in MenuItem.createHandle() OS.gtk_accel_label_set_accel_widget (label, 0); The "handle" is the default accel widget. > > > I am not sure there is any way to replace GTK_ACCEL_LABEL_GET_ACCEL_STRING > > using GTK API. The SWT and GTK APIs are different with respect to the > > accelerator text that shows up in the menu item (i. e Ctrl+A). SWT expects > > to set text which comes after the tab character in the menu item text (i.e > > MenuItem.setText()). GTK builds the accelerator text from the accelerator > > keycode and modifiers. GTK does not give any API to set the accelerator > > text directly. > > There should be some way around GTK_ACCEL_LABEL_GET_ACCEL_STRING ? All it > does is checks if there is something in the memory saved in "accel_string" > and clears it. The code basically replaces the accelerator text that is calculated by GTK with another text (provided by the SWT app). Run the snippet below. Note that the accelerator text (Ctrl+A) shows when running the old code, but it does not show with the new code. Eclipse never calls MenuItem.setAccelerator(). It does its own key binding by filtering key events. So Eclipse would not show any accelerators in menus with this patch. import org.eclipse.swt.*; import org.eclipse.swt.layout.*; import org.eclipse.swt.widgets.*; public class Snippet117 { public static void main(String[] args) { Display display = new Display(); Shell shell = new Shell(display); shell.setLayout(new FillLayout()); final Text t = new Text(shell, SWT.BORDER | SWT.MULTI); t.setText ("here is some text to be selected"); Menu bar = new Menu (shell, SWT.BAR); shell.setMenuBar (bar); MenuItem editItem = new MenuItem (bar, SWT.CASCADE); editItem.setText ("Edit"); Menu submenu = new Menu (shell, SWT.DROP_DOWN); editItem.setMenu (submenu); MenuItem item = new MenuItem (submenu, SWT.PUSH); item.addListener (SWT.Selection, new Listener () { public void handleEvent (Event e) { t.selectAll(); } }); item.setText ("Select &All\tCtrl+A"); // Comment/Uncoment this line // item.setAccelerator (SWT.MOD1 + 'A'); shell.setSize(200, 200); shell.open(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } display.dispose(); } }
(In reply to comment #3) > (In reply to comment #2) > > I am passing "handle" as the second parameter which is GtkWidget. I am just > > saying put label on the the "handle" widget. Why it is wrong ? > > Sorry, I misread the code. It is fine. You can achieve the same result by > commenting this line in MenuItem.createHandle() > > OS.gtk_accel_label_set_accel_widget (label, 0); > > The "handle" is the default accel widget. > > > > > > I am not sure there is any way to replace GTK_ACCEL_LABEL_GET_ACCEL_STRING > > > using GTK API. The SWT and GTK APIs are different with respect to the > > > accelerator text that shows up in the menu item (i. e Ctrl+A). SWT expects > > > to set text which comes after the tab character in the menu item text (i.e > > > MenuItem.setText()). GTK builds the accelerator text from the accelerator > > > keycode and modifiers. GTK does not give any API to set the accelerator > > > text directly. > > > > There should be some way around GTK_ACCEL_LABEL_GET_ACCEL_STRING ? All it > > does is checks if there is something in the memory saved in "accel_string" > > and clears it. > > The code basically replaces the accelerator text that is calculated by GTK > with another text (provided by the SWT app). Run the snippet below. Note > that the accelerator text (Ctrl+A) shows when running the old code, but it > does not show with the new code. Eclipse never calls > MenuItem.setAccelerator(). It does its own key binding by filtering key > events. So Eclipse would not show any accelerators in menus with this patch. > > > import org.eclipse.swt.*; > import org.eclipse.swt.layout.*; > import org.eclipse.swt.widgets.*; > > public class Snippet117 { > public static void main(String[] args) { > Display display = new Display(); > Shell shell = new Shell(display); > shell.setLayout(new FillLayout()); > final Text t = new Text(shell, SWT.BORDER | SWT.MULTI); > t.setText ("here is some text to be selected"); > Menu bar = new Menu (shell, SWT.BAR); > shell.setMenuBar (bar); > MenuItem editItem = new MenuItem (bar, SWT.CASCADE); > editItem.setText ("Edit"); > Menu submenu = new Menu (shell, SWT.DROP_DOWN); > editItem.setMenu (submenu); > > MenuItem item = new MenuItem (submenu, SWT.PUSH); > item.addListener (SWT.Selection, new Listener () { > public void handleEvent (Event e) { > t.selectAll(); > } > }); > item.setText ("Select &All\tCtrl+A"); > // Comment/Uncoment this line > // item.setAccelerator (SWT.MOD1 + 'A'); > > shell.setSize(200, 200); > shell.open(); > while (!shell.isDisposed()) { > if (!display.readAndDispatch()) > display.sleep(); > } > display.dispose(); > } > } Ok I definitely see that my patch works with setAccelerator but does not work with SetText. Maybe this question will look silly, but I am curious, why just not show the text that users passes as it is? Something like this in setText (line 892): char [] chars = fixMnemonic (string+accelString); or simply not to do substring of "string". Is it because tabs/spaces appear after Ctrl+A when you do it like this, or there are some other reasons ?
Setting the text with the accelerator portion is an option if we cannot find any better solution. One problem if this solution is that the accelerators should be right align in the menu. We could try to emulate the alignment by inserting spaces/tabs in between text|acceltext, but determining the number of spaces/tabs to insert is not simple and it depends on the font (monospaced vs proportional).
(In reply to comment #5) > Setting the text with the accelerator portion is an option if we cannot find > any better solution. > > One problem if this solution is that the accelerators should be right align > in the menu. We could try to emulate the alignment by inserting spaces/tabs > in between text|acceltext, but determining the number of spaces/tabs to > insert is not simple and it depends on the font (monospaced vs proportional). I have tried inserting spaces, it still does not work that way. One more thing that I cannot understand: Why Accelerator is added to setText() ? Why not to impose business rule that if you want to add Accelerator use setAccelerator () method after setText (), thus every method does what it initially intended to do. Looking at this snippet you showed before: When one looks at this setText () string it looks strange, isn't it ? Why putting '\t' and than Ctrl+A if you can add Accelerator label within its own method ? Way it is now: item.setText ("Select &All\tCtrl+A"); tem.setAccelerator (SWT.MOD1 + 'A'); Don't you think that code below looks cleaner ? item.setText ("Select &All"); item.setAccelerator (SWT.MOD1 + 'A'); Maybe I am missing some important part, of why it was done in the first place, as I didn't explored the whole class enough.
There are historical and technical reasons for the SWT API to be the way it is. Here are one of which: Historical: the first port of SWT was for Windows. The Windows API works exactly like the SWT API. Technical: the setAccelerator() API takes only one key stroke. It is not possible to implement multi-stroke key bindings. Eclipse wanted multi-stroke key bindings, so it stopped using the API and started filtering key events (Emacs bindings). Eclipse still needs to show what is the key binding in the Menu, so even though they do not call setAccelerator(), they still need a way to set the accelerator text. For example, open Eclipse preferences, filter for Keys, change Scheme to Emacs, click OK, open Edit menu. Note that the accelerator text for "Select All" is "Ctrl+X H".
We need to move on here. I have released code the stops calling these helpers for GTK_VERSION>=3.0.0. We need a solution to set the accelerator text on GTK 3, but for now the goal is to get the SWT libraries compiled. http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=1e698e4998cd0a3f8cc1c64d6ce4255ae7daf591
