Closed Bug 1252479 Opened 4 years ago Closed 4 years ago

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

Categories

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

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 47

People

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

References

Details

(Whiteboard: [btpp-backlog])

Attachments

(1 file)

have patch will travel
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.
Priority: -- → P3
Whiteboard: [btpp-backlog]
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"
Attachment #8725280 - Flags: review?(poirot.alex)
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");
Attachment #8725280 - Flags: review?(poirot.alex) → review+
(In reply to Pulsebot from comment #5)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/bcc5851a4598
I removed the tooltip changes before landing.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Depends on: 1257348
Depends on: 1259030
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.