Closed Bug 879845 Opened 6 years ago Closed 6 years ago

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

Categories

(Calendar :: Lightning: SeaMonkey Integration, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: philip.chee, Assigned: philip.chee)

References

(Blocks 1 open bug)

Details

(Keywords: calendar-integration)

Attachments

(2 files, 2 obsolete files)

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);
> +}
Attached patch Patch v1.0 (obsolete) — Splinter Review
> -  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
Attachment #758647 - Flags: review?(iann_bugzilla)
Comment on attachment 758647 [details] [diff] [review]
Patch v1.0

Have to think about this a bit more. Cancelling review request for the moment.
Attachment #758647 - Flags: review?(iann_bugzilla)
Attached file Patch v1.1 Better fix. (obsolete) —
> +  // 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.
Attachment #758647 - Attachment is obsolete: true
Attachment #759066 - Flags: review?(neil)
Attachment #759066 - Attachment is obsolete: true
Attachment #759066 - Attachment is patch: false
Attachment #759066 - Flags: review?(neil)
> +  // 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 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.
Attachment #759070 - Flags: review?(neil)
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?]
Attachment #759070 - Flags: review?(neil) → review+
Attachment #759070 - Attachment description: Patch v1.2 The correct patch this time. → Patch v1.2 [Fix in SeaMonkey]
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.
Attachment #760869 - Flags: review?(philipp)
Attachment #760869 - Flags: feedback?(bv1578)
Comment on attachment 760869 [details] [diff] [review]
Patch v2.0 Fix it in Lightning [check-n Comment 9].

Fine with me, r=philipp
Attachment #760869 - Flags: review?(philipp) → review+
Component: General → Lightning: SeaMonkey Integration
Product: SeaMonkey → Calendar
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
Attachment #760869 - Attachment description: Patch v2.0 Fix it in Lightning. → Patch v2.0 Fix it in Lightning [check-n Comment 9].
Target Milestone: --- → 2.6
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?
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.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Attachment #760869 - Flags: feedback?(bv1578)
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
Attachment #759070 - Attachment description: Patch v1.2 [Fix in SeaMonkey] → Patch v1.2 [Fix in SeaMonkey][check-in Comment 12]
You need to log in before you can comment on or make changes to this bug.