Closed Bug 1297546 Opened 4 years ago Closed 4 years ago

Title of Default Developer Tools in Toolbox Options should not hard-code the application name

Categories

(DevTools :: Framework, defect)

defect
Not set

Tracking

(firefox51 fixed)

RESOLVED FIXED
Firefox 51
Tracking Status
firefox51 --- fixed

People

(Reporter: rsx11m.pub, Assigned: rsx11m.pub)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

Bug 851546 introduced a Toolbox Options pane for the Developer Tools, which was since moved into common devtools/client code. At the time, the title of the pane (options.selectDefaultTools.label) was hard-coded as "Default Firefox Developer Tools" and thus doesn't show the branded name of the application as expected.

This entity should use "&brandShortName;" instead of "Firefox" to also reflect "Nightly" builds and especially non-Firefox applications like SeaMonkey which by now has DevTools included (bug 842942).
Attached patch Proposed patch (obsolete) — Splinter Review
This uses &brandShortName; in options.selectDefaultTools.label to show up correctly with the current application name. Note that this entity is only used in devtools/client/framework/toolbox-options.xhtml, and adding %brandDTD; there seems to be sufficient despite toolbox.dtd being included in other files too.

Note that there is also options.enableRemote.tooltip using "Firefox" as string, however within the scope of an example, thus I've left it untouched there.
Assignee: nobody → rsx11m.pub
Status: NEW → ASSIGNED
Attachment #8784179 - Flags: review?(jryans)
Comment on attachment 8784179 [details] [diff] [review]
Proposed patch

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

There is ongoing work as part of converting DevTools to HTML that removes .dtd files and switches .properties everywhere (bugs filed under bug 1290947).

:jdescottes, can you take a look at this?  I am not sure if toolbox-options.xhtml is in scope for the conversion, and if it, you may prefer a different approach.  I am not sure if something like &brandShortName is accessible for properties.  As a workaround, the brand name could be dropped from the string entirely.
Attachment #8784179 - Flags: review?(jryans) → review?(jdescottes)
As far as I can tell, brand.properties contains brandShortName as a string property, so that should not be an obstacle for such a conversion.
Comment on attachment 8784179 [details] [diff] [review]
Proposed patch

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

I think we should simply drop the brand name here. 

If I look at the various values for brandShortName, we have "Firefox Developer Edition" for aurora, and I don't think we want to have "Default Firefox Developer Edition Developer Tools" as a thing here :) Alternatively, there is brandShorterName, but it doesn't seem to be available for all brands, so I wouldn't use this either.
I would simply go for "Default Developer Tools", what do you think? 

I won't ask you to move this to a properties file and translate it in toolbox-options.js here. You are just updating an existing string, and toolbox-options.xul will probably be easier to migrate all at once.
Attachment #8784179 - Flags: review?(jdescottes)
(In reply to Julian Descottes [:jdescottes] from comment #4)
> If I look at the various values for brandShortName, we have "Firefox
> Developer Edition" for aurora, and I don't think we want to have "Default
> Firefox Developer Edition Developer Tools" as a thing here :)

Agreed, that looks funny indeed.

> there is brandShorterName, but it doesn't seem to be available for all
> brands, so I wouldn't use this either.

SeaMonkey at least doesn't have it, right.

> I would simply go for "Default Developer Tools", what do you think?

This looks like the best option then. I'd also shorten "debug remote Firefox instance like Firefox OS" to just "a remote instance" in options.enableRemote.tooltip to make that string application neutral, unless there are any objections.
 
> I won't ask you to move this to a properties file and translate it in
> toolbox-options.js here. You are just updating an existing string, and
> toolbox-options.xul will probably be easier to migrate all at once.

Thanks, yes - that would be beyond the scope I've filed this bug for. ;-)
Summary: Title of Default Developer Tools in Toolbox Options should use branded entity rather than hardcode the application name → Title of Default Developer Tools in Toolbox Options should not hard-code the application name
Attached patch Proposed patch (v2) (obsolete) — Splinter Review
Modifications per Comment #5.
Attachment #8784179 - Attachment is obsolete: true
Attachment #8784565 - Flags: review?(jdescottes)
Comment on attachment 8784565 [details] [diff] [review]
Proposed patch (v2)

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

Thanks! Just one minor issue to fix, good to go after that.
r=me after that.

::: devtools/client/locales/en-US/toolbox.dtd
@@ +118,5 @@
>    -  tooltip for the checkbox that toggles the service workers testing features on or off. This option enables service workers over HTTP. -->
>  <!ENTITY options.enableServiceWorkersHTTP.label     "Enable Service Workers over HTTP (when toolbox is open)">
>  <!ENTITY options.enableServiceWorkersHTTP.tooltip   "Turning this option on will enable the service workers over HTTP for all tabs that have the toolbox open.">
>  
>  <!-- LOCALIZATION NOTE (options.selectDefaultTools.label): This is the label for

Update the localization note options.selectDefaultTools.label -> options.selectDefaultTools.label2
Attachment #8784565 - Flags: review?(jdescottes) → review+
(In reply to Julian Descottes [:jdescottes] from comment #7)
> Update the localization note options.selectDefaultTools.label ->
> options.selectDefaultTools.label2

Sorry, missed that. Ready to land now.
Attachment #8784565 - Attachment is obsolete: true
Attachment #8784989 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/cc41a931cbf3
Title of Default Developer Tools in Toolbox Options should not hard-code the application name. r=jdescottes
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/cc41a931cbf3
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
I have reproduced this bug with Nightly 51.0a1 (2016-08-23) on Windows 7 , 64 Bit !

This bug's fix is verified with latest Nightly   

Build ID    20161012030211
User Agent  Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0
[bugday-20161012]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.