Port Bug 1465219 to C-C: Replace MenuBoxObject with XULElement subclass

RESOLVED FIXED in Thunderbird 64.0

Status

enhancement
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: jorgk, Assigned: jorgk)

Tracking

unspecified
Thunderbird 64.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

Assignee

Description

9 months ago
Looking at https://hg.mozilla.org/mozilla-central/rev/6dd5d0b6ba70 there seem to be two patterns we need to replace:

-menulist.menuBoxObject.activeChild = currentOption;
+menulist.activeChild = currentOption;

and

-aMenuBtn.boxObject.openMenu(true);
+aMenuBtn.openMenu(true);

Let me try that on C-C.
Assignee

Comment 2

9 months ago
Oops, |instanceof MenuBoxObject| needed replacement, too. And |ChromeUtils.getClassName(targetNode) == "XULElement"| only found in suite/.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a46faa3162938e3c3de9e30f37a6c3df89c1f1ad

FRG, should I land the suite/ changes here or do you prefer another bug?
Attachment #9010342 - Attachment is obsolete: true
Flags: needinfo?(frgrahl)

Comment 3

9 months ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/91c8e34b5a10
Port Bug 1465219: Replace MenuBoxObject with XULElement subclass. rs=bustage-fix
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Assignee

Comment 4

9 months ago
Sadly both tries are green, so back to manual testing.

Editable menulists:
- Edit from address, fiddled with this, eventually I saw:
  this.menuBoxObject is undefined; can't access its "activeChild" property
  The patch fixes that.

The code in mailWidgets.xml is for "Folder picker helper widgets". Sorry, I can't find a way to trigger that.

And the date picker isn't used in TB, only in add-ons :-(
Assignee

Comment 5

9 months ago
Comment on attachment 9010346 [details] [diff] [review]
1492522-MenuBoxObject.patch (v2)

OK, I've landed the TB part only. One hunk was certainly correct, not so sure about the other two which I couldn't test.
Flags: needinfo?(frgrahl)
Attachment #9010346 - Flags: review?(frgrahl)
Attachment #9010346 - Flags: review?(acelists)
Assignee

Updated

9 months ago
Target Milestone: --- → Thunderbird 64.0
Assignee

Comment 6

9 months ago
Posted file datepicker-2.0.xpi
This is Jonathan Kamens "Datepicker Bug Reproducer" add-on ported to legacy WE. Somehow it doesn't automatically load the panel, bu that can be done in the error console with:
openDialog("chrome://dtpr/content/dialog.xul", "_blank", "chrome,titlebar,modal,resizable", null);

The datepicker works (sort of, the month doesn't change in the pop-up). Without the patch I get:
JavaScript error: chrome://messenger/content/datetimepicker.xml, line 1262: ReferenceError: MenuBoxObject is not defined. With the patch there's no such error.
Assignee

Comment 7

9 months ago
Filed bug 1492560 for the month that doesn't change.

Comment 8

9 months ago
Comment on attachment 9010346 [details] [diff] [review]
1492522-MenuBoxObject.patch (v2)

Review of attachment 9010346 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, seems to match what m-c has done.

::: suite/browser/nsTypeAheadFind.js
@@ +158,5 @@
>          element.isContentEditable)
>        return true;
>  
>      // Don't start a find if the focus is on a form element.
> +    if (isXULElement(element) ||

Will this work anywhere? m-c only implemented isXULElement in accessible/tests/mochitest/events.js .
Attachment #9010346 - Flags: feedback+
Assignee

Comment 9

9 months ago
Posted patch landed.patchSplinter Review
Can I just have a review of the landed patch?
Attachment #9010419 - Flags: review?(acelists)
Assignee

Updated

9 months ago
Attachment #9010346 - Flags: review?(acelists)
Assignee

Comment 10

9 months ago
This is the suite/ part. Yes, isXULElement() wouldn't have worked, I inlined the function here.
Attachment #9010346 - Attachment is obsolete: true
Attachment #9010346 - Flags: review?(frgrahl)
Attachment #9010428 - Flags: review?(frgrahl)
Attachment #9010428 - Flags: feedback?(acelists)

Comment 11

9 months ago
Comment on attachment 9010419 [details] [diff] [review]
landed.patch

Review of attachment 9010419 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks
Attachment #9010419 - Flags: review?(acelists) → review+
Comment on attachment 9010428 [details] [diff] [review]
1492522-MenuBoxObject.patch - suite/ part only

Can't test right now becuase we are way behind with fixing but look great. r+
Attachment #9010428 - Flags: review?(frgrahl) → review+

Comment 13

9 months ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/5fca7138f5f6
Port Bug 1465219: Replace MenuBoxObject with XULElement subclass in suite/. r=frg

Updated

9 months ago
Attachment #9010428 - Flags: feedback?(acelists) → feedback+
You need to log in before you can comment on or make changes to this bug.