Closed Bug 1223344 Opened 4 years ago Closed 4 years ago

Some tweaks to gDevTools.jsm to get Devtools working in SeaMonkey

Categories

(DevTools :: General, defect)

defect
Not set

Tracking

(firefox45 fixed)

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- fixed

People

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

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Assignee: nobody → philip.chee
Status: NEW → ASSIGNED
Blocks: 1223341
No longer depends on: 1223341
> -    let tabContainer = win.document.getElementById("tabbrowser-tabs");
> +    let tabContainer = win.gBrowser.tabContainer;
Devtools uses gBrowser.tabContainer 48 times getElementById("tabbrowser-tabs") 2 times.
SeaMonkey's tabbrowser has a tabContainer property, but has a different elementID for the "tabbrowser-tabs".

> -    let mp = doc.getElementById("menuWebDeveloperPopup");
>      let mps = doc.getElementById("menu_devtools_separator");
> -    mp.insertBefore(fragMenuItems, mps);
> +    if (mps)
> +      mps.parentNode.insertBefore(fragMenuItems, mps);

For historical reasons our menupopup is called "toolsPopup" which we don't want to change due to backward compatibility with addons. On the other hand you can avoid needing the menupopup by doing:

let foo = doc.getElementById("foo_separator");
foo.parentNode.insertBefore(bar, foo);

>            let tabContainer = win.gBrowser.tabContainer;
> -          let numPinnedTabs = tabContainer.tabbrowser._numPinnedTabs;
> +          let numPinnedTabs = win.gBrowser._numPinnedTabs || 0;

(1) gBrowser is the tabbrowser. So you use gBrowser to get the tabContainer, then use the tabContainer to get the tabbrowser. This makes no sense.
(2) SeaMonkey's tabbrowser doesn't have a _numPinnedTabs property so this ends up as undefined. We can coerce this to a number using ||0 . 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0616fad56376&group_state=expanded
https://treeherder.mozilla.org/#/jobs?repo=try&revision=37e7d13f6855&group_state=expanded
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6c0342c56d4&group_state=expanded
Attachment #8685404 - Attachment is obsolete: true
Attachment #8690912 - Flags: review?(jryans)
Comment on attachment 8690912 [details] [diff] [review]
Patch v1.1 tweak gDevTools.jsm for SeaMonkey compat

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

::: devtools/client/framework/gDevTools.jsm
@@ +1085,5 @@
>      mbs.appendChild(fragBroadcasters);
>  
> +    let amps = doc.getElementById("appmenu_devtools_separator");
> +    if (amps)
> +      amps.parentNode.insertBefore(fragAppMenuItems, amps);

Nit: DevTools style uses braces even with 1 line if blocks

@@ +1091,3 @@
>      let mps = doc.getElementById("menu_devtools_separator");
> +    if (mps)
> +      mps.parentNode.insertBefore(fragMenuItems, mps);

Nit: DevTools style uses braces even with 1 line if blocks
Attachment #8690912 - Flags: review?(jryans) → review+
https://hg.mozilla.org/mozilla-central/rev/e091924468af
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.