Open Bug 974824 Opened 10 years ago Updated 3 months ago

Remove bogus sizetopopup attributes

Categories

(Toolkit :: Autocomplete, defect, P3)

defect

Tracking

()

ASSIGNED

People

(Reporter: neil, Assigned: neil)

References

Details

Attachments

(2 files)

All in-tree autocomplete widgets have a <content sizetopopup="pref"> attribute. This attribute seems to have existed solely so because the XPFE history menu was depending on an obscure XUL layout bug in that the history popup would size itself to the autocomplete widget despite not being a menu. The attribute was then cargo-culted to the toolkit autocomplete widget.

As of bug 974258 the sizetopopup="pref" only applies to actual menus, typically <menulist> elements. The bogus attribute should therefore be removed from other elements.

The XPFE history menu issue is being addressed in bug 974258.
Attached patch Proposed patchSplinter Review
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #8378883 - Flags: review?(enndeakin)
> As of bug 974258 the sizetopopup="pref" only applies to actual menus,
I mean 941409 of course.
Depends on: 941409
No longer depends on: 974258
Attachment #8378883 - Flags: review?(enndeakin) → review+
Try run confirms it was this patch, but I have no idea why the sizetopopup attribute comes into play here.
OK, so what's happening here is that nsMenuPopupFrame::IsLeaf cautiously decides that it's not a leaf if its parent element has the sizetopopup attribute, despite not needing to be a leaf unless its parent frame is a menu frame and the sizetopopup attribute being either "always" or "pref".

This means that the a11y tree test looks for the child of the menupopup because its parent happens to have that attribute.

I'm going to go out on a limb and suggest the test was written against what you observed at the time, i.e. garbage in, garbage out ;-)
Attached patch Possible patchSplinter Review
So, I went one step further and improved the code that looks for the bogus sizetopopup attribute to behave like the layout code would, so that it only takes effect if the parent frame is a menu.

This now appears to expose the context menu accessibles eagerly, which confuses test_combobox.xul; is this a bug in the test or somewhere else?
Attachment #8380395 - Flags: feedback?(surkov.alexander)
Attachment #8380395 - Flags: feedback?(enndeakin)
Attachment #8380395 - Flags: feedback?(enndeakin) → feedback+
It's hard to say what test exactly fails since full log says "(elided 100 passes or known failures)" and doesn't provide enough context. Presumably it was "autocomplete" test though. Anyway regardless which test fails it sounds like the problem is frame tree is not generated under menupopup anymore, that would prevent the creation of accessible tree.
Comment on attachment 8380395 [details] [diff] [review]
Possible patch

I'm not sure how else I can help, that could be a test fix or a bug, the it seems like the accessible tree/frame tree is not created while the test assumes it should be presented.
Attachment #8380395 - Flags: feedback?(surkov.alexander)
Priority: -- → P3
Severity: trivial → S4
You need to log in before you can comment on or make changes to this bug.