Closed Bug 277750 Opened 16 years ago Closed 15 years ago
Menu exposes 2-level accessible
By AT-Poke, there are 2-level accessible for menu. One is accessible of XUL menu; the other is accessible of XUL menupopup. From the point of view of end user, we should keep only one, and the children of menu accessible should be menu item accessible.
*** Bug 249104 has been marked as a duplicate of this bug. ***
(In reply to comment #0) > By AT-Poke, there are 2-level accessible for menu. One is accessible of XUL > menu; the other is accessible of XUL menupopup. From the point of view of end > user, we should keep only one, and the children of menu accessible should be > menu item accessible. I have tested this patch on a build from today, and I still see two levels of menus in at-poke. Does this patch include all of the changes to fix this problem?
Sorry for missing the fix in xpfe directory. After some testing, I found the patches for menu/popup menu in my hand are hard to verify against the current trunk. New problems may come out after applying the patch. I am not very clear about the reason. It may be blocked by other fixes, or due to the difference between 1.7 branch and trunk. This patch is posted just for refernece. It don't need to be "reviewed". I'll take care of this one after finishing other porting.
*** Bug 319752 has been marked as a duplicate of this bug. ***
Most of patch v2 were already commited by Bug 257099. Need to port xpfe part to toolkit/content/widgets
Comment on attachment 205708 [details] [diff] [review] port xpfe part of bug 257099 to toolkit Needs to be reviewed by toolkit peer as well. Neil is both an sr= and a toolkitp peer, so he's a good choice.
Comment on attachment 205708 [details] [diff] [review] port xpfe part of bug 257099 to toolkit >+ if (this.localName == "popup") > return (this.parentNode.localName == "menulist")? accService.createXULSelectListAccessible(this): accService.createXULMenupopupAccessible(this); >+ // >+ // Create accessible for menupopup when parent node is: >+ // 1. menulist: for combobox to visit its menu items >+ // 2. toolbarbutton: for which has menupopup child >+ // Otherwise, won't create accessible for menupopup, e.g. >+ // 1. menu: to avoid redundant accessible for menu >+ // 2. other situations ... >+ // >+ if (this.localName == "menupopup") >+ return ((this.parentNode.localName == "menulist") || >+ (this.parentNode.localName == "toolbarbutton")) ? >+ accService.createXULMenupopupAccessible(this): null; I'm confused. What difference does it make if this is a popup or a menupopup? Apart from tooltips, there are several ways (menu)popups get used: a) attached to a parent menu, in case I understand that you want to return null b) attached to a parent menulist, which has a special meaning as a selection list c) attached to a (toolbar)button, colour picker, column picker d) not specifically attached to anything - used as a context menu Note that it is difficult to distinguish between c) and d) especially when comparing (say) <button type="menu"> with <button context="_child">. Needless to say, if this patch gets changed to clarify the picture then the xpfe version should get changed too!
only care about the case when parent node is menu at this moment
Attachment #206838 - Flags: superreview?(neil.parkwaycc.co.uk)
Comment on attachment 206838 [details] [diff] [review] patch v3 >+ // Won't create accessible for (menu)popup if parent node is menu >+ // to avoid redundant accessible for menu >+ if (this.localName == "popup" || this.localName == "menupopup") >+ return (this.parentNode.localName != "menu")? > accService.createXULMenupopupAccessible(this): null; > else if (this.localName == "tooltip") > return accService.createXULTooltipAccessible(this); > return null; Minor nit: ideally have a space before the ? I feel sure it's technically possible to rearrange the conditions to avoid having to use the ?: (and indeed there should not be an else after the first return) but thinking about it further I guess tooltip should have its own <implementation><property> anyway so don't bother.
Attachment #206838 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment on attachment 206838 [details] [diff] [review] patch v3 Sync Firefox and Seamonkey. Risk is low.
Attachment #206838 - Flags: approval1.8.1?
Attachment #206838 - Flags: approval1.8.1? → branch-1.8.1?(aaronleventhal)
Aaron, I have tested the following bug 277750 for any regress on the levels below & updated the bug in bugzilla. FF 3 1.6a1 build date 03/12/06 JAWS beta 7.10.245 03/10/06 Window Eyes 5.5 beta The areas tested were within Firefox 3 are: 1. combo boxes in the UI 2. context menus 3. main menu bar menus
Attachment #206838 - Flags: approval-branch-1.8.1?(aaronleventhal) → approval-branch-1.8.1+
Whiteboard: sunport17 → [checkin needed (1.8 branch)] sunport17
Checking in toolkit/content/widgets/menu.xml; /cvsroot/mozilla/toolkit/content/widgets/menu.xml,v <-- menu.xml new revision: 184.108.40.206; previous revision: 220.127.116.11 done Checking in toolkit/content/widgets/popup.xml; /cvsroot/mozilla/toolkit/content/widgets/popup.xml,v <-- popup.xml new revision: 18.104.22.168; previous revision: 1.4 done Checking in xpfe/global/resources/content/bindings/popup.xml; /cvsroot/mozilla/xpfe/global/resources/content/bindings/popup.xml,v <-- popup.xml new revision: 22.214.171.124; previous revision: 1.27 done
What end-user problem did this fix? It caused a problem for MSAA. Since there is now no accessible for the menus in the menubar, there is no accessible to fire a menupopupstart/menupopupend event and thus nothing gets fired.
It appears that MSAA and ATK unfortunately structure menus differently: ATK: File ( role = menu) New (role = menu item) Open (role = menu item) MSAA: File (role = menuitem) File (role = menupopup) New (role = menu item) Open (role = menu item)
Without this patch, user have to click twice for the first level of menu in GOK i.e. Menus->File->File->Quit
You need to log in before you can comment on or make changes to this bug.