Bug 1108761 - Add class to let menulist > menupopup stylings apply to other popups

So Dao's idea is to add a class, but instead of duplicating the class for every rule, have the class supplant the child rule, and have the menulist binding apply the class on the menupopup when it opens. Dao said popup.xml, but I think he means menulist.xml.

Ok, that sounds pretty straight forward. Let's get started.

Places I'm likely going to need to update:

toolkit/themes/[platform]/global/popup.css

toolkit/themes/[platform]/global/global.css

toolkit/themes/[platform]/global/menu.css

Garrr… this isn't going to be so simple. Some of the rules look like this:

menulist[editable="true"] > menupopup > […] {

/** rule **/

}

So we need to know whether or not the menulist is editable...

So maybe we actually need two classes:

editable-menulist-menupopup

uneditable-menulist-menupopup

Or… or just have the editable attribute get inherited by the child. Can that even happen? Not on <children> it seems. Hrm.

Ok, so how about we have the menulist also copy the editable attribute on the menupopup when it opens it? Just like the class.

That way, we have one class, and one maybe attribute. Let's try that.

:not(menulist) > menupopup > menuitem[checked="true"],

}

Tricky - the leftmost part of both selectors says:

"Any menupopup that is not a child of a menulist… apply rule"

However, we want to modify these to be:

"Any menupopup that is not a child of a menulist AND any menupopup that does not have the .menulist-menupopup class… apply rule"

Actually, since we can guarantee that the menulist will add the menulist-menupopup class, we can simply say:

"Any menupopup that does not have the menulist-menupopup class… apply rule". Simpler than I had thought.

menupopup:not(.menulist-menupopup) > menuitem[checked="true"],

menupopup:not(.menulist-menupopup) > menuitem[selected="true"] {

/* foo */

}

Hrmph. Due to specificity, I had to add a rule for the _moz-menuactive attribute so that the text gets highlighted properly. Lovely.

Ok, getting things going for Linux now...

Don't forget the inContent stuff

Fix weird checkbox thing on Linux

CHANGE OF PLANS.

I came up with something that I think is a simpler solution to this problem. I've been assuming that I can't use a standard menulist because I would have the top of the menulist to deal with. But now I've added a new type of menulist to XUL, with a popuponly="true" attribute that hides the top-bit. Now the menulist on the parent side keeps track of list state, which is nice. No more hackery - since the popup really expects to be inside some type of list.

Dao thinks the way I'm hiding the top bit (the part that's usually clicked for menulists) can be done better… I'm using height: 0 !important, margin: 0 !important, border: 0 !important, to reduce the height and eliminate it's visibility from the DOM. Dao thinks I can do it by setting visibility: collapse on the parent and visibility: visible on the menupopup, but I couldn't get it to work. I've given him the patch to play with.

FUCK. Failing tests.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=97a392cd9f9d

Huh. Only fails on Windows and Linux. That's great.

dom/tests/mochitest/chrome/test_focus.xul is what's failing.

821 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/chrome/test_focus.xul | frameset tab key wrap to start events - got blur: f8 blur: frame-f7-document blur: frame-f7-window, expected blur: f8 blur: frame-f7-document blur: frame-f7-window focus: frame-f1-document focus: frame-f1-window

822 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/chrome/test_focus.xul | frameset tab key wrap to start focusedElement - got ContentSelectDropdown, expected f1

823 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/chrome/test_focus.xul | frameset tab key wrap to start focusedWindow - got [object ChromeWindow], expected [object Window]

824 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/chrome/test_focus.xul | frameset tab key wrap to start getFocusedElementForWindow - got [object HTMLInputElement], expected [object HTMLHtmlElement]

826 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/chrome/test_focus.xul | frameset tab key wrap to start hasFocus - got false, expected true

827 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/chrome/test_focus.xul | frameset tab key wrap to start activeElement - got f2, expected f1

828 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/chrome/test_focus.xul | frameset tab key wrap to start commandDispatcher focusedWindow - got [object ChromeWindow], expected [object Window]

830 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/chrome/test_focus.xul | frameset shift tab key wrap to end events - got focus: frame-f7-document focus: frame-f7-window focus: f8, expected blur: frame-f1-document blur: frame-f1-window focus: frame-f7-document focus: frame-f7-window focus: f8

929 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/chrome/test_focus.xul | switch document forward and wrap events - got blur: frame-f7-document blur: frame-f7-window, expected blur: frame-f7-document blur: frame-f7-window focus: frame-f1-document focus: frame-f1-window

930 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/chrome/test_focus.xul | switch document forward and wrap focusedElement - got ContentSelectDropdown, expected f1

931 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/chrome/test_focus.xul | switch document forward and wrap focusedWindow - got [object ChromeWindow], expected [object Window]

934 INFO TEST-UNEXPECTED-FAIL | dom/tests/mochitest/chrome/test_focus.xul | switch document forward and wrap hasFocus - got false, expected true

Ah, OK. So having the ContentSelectDropdown not be hidden immediately causes it to be focusable when we don't want it to be. I'll set hidden="true" on it by default, and only remove that hidden="true" whenever we want the <select> dropdown to open. Piece of cake. Phew.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3322f4157cf7

Posted for review. Hopefully Enn gets to it soon!

Woo! Enn r+'d me - though he wants to know why the menulist isn't automatically scrolling the selected menuitem into view - apparently, nsMenuPopupFrame::EnsureMenuItemIsVisible is supposed to be called. He wants to know why this doesn't work.

I'll look into it.

We do hit nsMenuPopupFrame::EnsureMenuItemIsVisible, and then go into PresShell::ScrollFrameRectIntoView. We climb the ancestor chain until we hit an nsXULScrollFrame, and then call ScrollToShowRect, which calls ComputeNeedToScroll...

I suspect this is because our <xul:menulist> item has no rect to speak of.

Landed. Done.