879845 - Lightning Bug 718332 broke SeaMonkey's customize toolbar context menu.

Status: RESOLVED FIXED

(Calendar :: Lightning: SeaMonkey Integration, defect)

Description

[Philip Chee]

Thunderbirds onViewToolbarsPopupShowing() takes three parameters (aEvent, toolbox, aInsertPoint) with aInsertPoint being the third parameter. In contrast SeaMonkey follows the Firfox API and puts aInsertPoint as the second parameter of two.

http://hg.mozilla.org/comm-central/rev/6ea5e19539b8#l3.16

> +function onToolbarsPopupShowingWithMode(aEvent, aInsertPoint) {
> +    let toolbox = [gCurrentMode + "-toolbox"];
> +    if (gCurrentMode != "mail") {
> +        toolbox.push("navigation-toolbox");
> +    }
> +    onViewToolbarsPopupShowing(aEvent, toolbox, aInsertPoint);
> +}

Comment 1

[Philip Chee]

Attachment: Patch v1.0

> -  var firstMenuItem = aInsertPoint || popup.firstChild;
> -
> +  var firstMenuItem = aInsertPoint instanceof XULElement ? aInsertPoint
> +                                                         : popup.firstChild;

Check that the second parameter is a XULElement. Thunderbird sends an array as the second parameter.

This fixes:

> Thu Jun 06 2013 00:33:28
> Error: TypeError: Value does not implement interface Node.
> Source file: chrome://communicator/content/utilityOverlay.js
> Line: 379

Comment 2

[Philip Chee]

Comment on attachment 758647 [details] [diff] [review]
Patch v1.0

Have to think about this a bit more. Cancelling review request for the moment.

Comment 3

[Philip Chee]

Attachment: Patch v1.1 Better fix.

> +  // Thunderbird/Lightning function signature is:
> +  // onViewToolbarsPopupShowing(aEvent, toolboxIds, aInsertPoint)
> +  // where toolboxIds is either a string or an array of strings.

Any caller with three arguments is coming from Lightning or some other Thunderbird extension. With two arguments it becomes slightly more tricky so we just check if the second argument is a XULElement or not.

Comment 4

[Philip Chee]

Attachment: Patch v1.2 [Fix in SeaMonkey][check-in Comment 12]

> +  // Thunderbird/Lightning function signature is:
> +  // onViewToolbarsPopupShowing(aEvent, toolboxIds, aInsertPoint)
> +  // where toolboxIds is either a string or an array of strings.

Any caller with three arguments is coming from Lightning or some other Thunderbird extension. With two arguments it becomes slightly more tricky so we just check if the second argument is a XULElement or not.

Comment 5

[Philip Chee]

Comment on attachment 759070 [details] [diff] [review]
Patch v1.2 [Fix in SeaMonkey][check-in Comment 12]

Getting this fixed on the Thunderbird/Lightning side of things looks a bit messy. And I don't have the time to unravel the TB usage.

Comment 6

[[e-mail]]

Comment on attachment 759070 [details] [diff] [review]
Patch v1.2 [Fix in SeaMonkey][check-in Comment 12]

>+  if (arguments[2])
>+    aInsertPoint = arguments[2];
The third argument is only an insertion point for the Thunderbird app menu, which is irrelevant here, so we don't need to check for it, so you just need the instanceof check. r=me with that fixed.

The alternative is to overlay lightning-menus.xul to undo the onpopupshowing attribute overwriting code...

[How many toolboxes does Thunderbird have?]

Comment 7

[Philip Chee]

Attachment: Patch v2.0 Fix it in Lightning [check-n Comment 9].

This patch addresses the issue on the Lightning side of things. It does the minimum to unbreak SeaMonkey and doesn't add any Lightning toolbars to the SeaMonkey View->Show/Hide menu.

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 760869 [details] [diff] [review]
Patch v2.0 Fix it in Lightning [check-n Comment 9].

Fine with me, r=philipp

Comment 9

[Philip Chee]

Comment on attachment 760869 [details] [diff] [review]
Patch v2.0 Fix it in Lightning [check-n Comment 9].

Lightning patch pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/42ffb0f73c89

Comment 10

[Stefan Sitter [:ssitter]]

Philip, what is the status of this bug? Seems that your Patch v2.0 was checked in. Is Patch v1.2 still required? Or is it obsoleted by Patch v2.0?

Comment 11

[Philip Chee]

Yes. Sorry. This bug is fixed with Lightning Patch v2.0. Patch v1.2 is just some defensive programming which I intend to check-in after the SeaMonkey part of comm-central re-opens. I'll close this to take it off your queries.

Comment 12

[Philip Chee]

Comment on attachment 759070 [details] [diff] [review]
Patch v1.2 [Fix in SeaMonkey][check-in Comment 12]

Pushed SeaMonkey part to comm-central:
http://hg.mozilla.org/comm-central/rev/ec7c6d8af7d8