1252479 - Adding the Developer Dynamically (Bug 1248601) doesn't work in SeaMonkey because it doesn't have a "browser-bottombox"

Status: RESOLVED FIXED

(DevTools Graveyard :: Graphic Commandline and Toolbar, defect, P3)

Description

[Philip Chee]

have patch will travel

Comment 1

[Philip Chee]

Attachment: Patch v1 adjust for SeaMonkey not having a "browser-bottombox"

Also tweaked the close button on the developer toolbar to use the styles in global.css so that it will match whatever third party theme has been installed.

Comment 2

[Philip Chee]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7f02023cde4f&exclusion_profile=false&group_state=expanded

Comment 3

[Philip Chee]

Comment on attachment 8725280 [details] [diff] [review]
Patch v1 adjust for SeaMonkey not having a "browser-bottombox"

This patch also includes some "improvements" I made on the SeaMonkey side:

> -  close.setAttribute("class", "devtools-closebutton");
> +  close.setAttribute("class", "close-icon");
Use toolkit close button styles so will pick up styles from third party themes or OS platform.

>    toolboxBtn.setAttribute("tooltiptext", "devToolbarToolsButton.tooltip");
> +  toolboxBtn.setAttribute("_defaultTooltipText", "devToolbarToolsButton.tooltip");
I hope you realise that the default tooltip text is litterally "devToolbarToolsButton.tooltip"

Comment 4

[Alexandre Poirot [:ochameau] [PTO back August 21]]

Comment on attachment 8725280 [details] [diff] [review]
Patch v1 adjust for SeaMonkey not having a "browser-bottombox"

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

Looks good and yes, the tooltip is broken :/

::: devtools/client/shared/developer-toolbar.js
@@ +318,5 @@
>    let toolboxBtn = this._doc.createElement("toolbarbutton");
>    toolboxBtn.setAttribute("id", "developer-toolbar-toolbox-button");
>    toolboxBtn.setAttribute("class", "developer-toolbar-button");
>    toolboxBtn.setAttribute("observes", "devtoolsMenuBroadcaster_DevToolbox");
>    toolboxBtn.setAttribute("tooltiptext", "devToolbarToolsButton.tooltip");

It should be "&devToolbarToolsButton.tooltip".
Same for next line, but I'm not sure dtd works when adding xul dynamically, that way.
I imagine we have to convert this dtd to properties file.
Feel free to keep that in another bug.

@@ +319,5 @@
>    toolboxBtn.setAttribute("id", "developer-toolbar-toolbox-button");
>    toolboxBtn.setAttribute("class", "developer-toolbar-button");
>    toolboxBtn.setAttribute("observes", "devtoolsMenuBroadcaster_DevToolbox");
>    toolboxBtn.setAttribute("tooltiptext", "devToolbarToolsButton.tooltip");
> +  toolboxBtn.setAttribute("_defaultTooltipText", "devToolbarToolsButton.tooltip");

nit: There is no need it to be a DOM Attribute,
it could be just a JS property:
  toolboxBtn._defaultTooltipText = toolboxBtn.getAttribute("tooltiptext");

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bcc5851a4598

Comment 6

[Philip Chee]

(In reply to Pulsebot from comment #5)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/bcc5851a4598
I removed the tooltip changes before landing.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bcc5851a4598