Closed Bug 1192523 Opened 10 years ago Closed 8 years ago

"(cached)" tooltip of request method in netmonitor cannot be localized

Categories

(DevTools :: Netmonitor, defect)

Product:
DevTools
Component:
Netmonitor
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: stef, Assigned: vkatsikaros)

Details

Attachments

(2 files)

Bug 1192523 - "(cached)" tooltip of request method in netmonitor cannot be localized.
59 bytes, text/x-review-board-request
Honza
: review+
Details
status_tooltip.png
14.28 KB, image/png
Details
No description provided.
https://hg.mozilla.org/mozilla-central/file/2ee3ba67281f/browser/devtools/netmonitor/netmonitor-view.js#l1466
Summary: "(cached)" in → "(cached)" tooltip of request method in netmonitor cannot be localized
Assignee: nobody → vkatsikaros
Comment on attachment 8858742 [details] Bug 1192523 - "(cached)" tooltip of request method in netmonitor cannot be localized. https://reviewboard.mozilla.org/r/130776/#review133308 ::: devtools/client/locales/en-US/netmonitor.properties:813 (Diff revision 1) > # next to a header list item, with a link to external documentation > netmonitor.headers.learnMore=Learn More > > +# LOCALIZATION NOTE (netmonitor.status.cached): This is appended in the tooltip > +# for the value of the column status code, when the request is cached > +netmonitor.status.cached= (cached) The leading space is ignored, so I used a template literal in the .js file.
Comment on attachment 8858742 [details] Bug 1192523 - "(cached)" tooltip of request method in netmonitor cannot be localized. https://reviewboard.mozilla.org/r/130776/#review133312
Comment on attachment 8858742 [details] Bug 1192523 - "(cached)" tooltip of request method in netmonitor cannot be localized. https://reviewboard.mozilla.org/r/130776/#review133318 ::: devtools/client/locales/en-US/netmonitor.properties:813 (Diff revision 1) > # next to a header list item, with a link to external documentation > netmonitor.headers.learnMore=Learn More > > +# LOCALIZATION NOTE (netmonitor.status.cached): This is appended in the tooltip > +# for the value of the column status code, when the request is cached > +netmonitor.status.cached= (cached) It would make more sense to have a string like this than hard-coding spaces and concatenations netmonitor.status.cached = %S (cached)
Comment on attachment 8858742 [details] Bug 1192523 - "(cached)" tooltip of request method in netmonitor cannot be localized. https://reviewboard.mozilla.org/r/130776/#review133318 > It would make more sense to have a string like this than hard-coding spaces and concatenations > netmonitor.status.cached = %S (cached) I thought about that. However, if I understand correctly, the strings are appended independently to the title, so it would take 2 localized strings `%S (cached)` and `% (cached) (service worker)`, and lots more if extra ones are added in the future. Happy to go either direction.
(In reply to Vangelis Katsikaros from comment #6) > I thought about that. However, if I understand correctly, the strings are > appended independently to the title, so it would take 2 localized strings > `%S (cached)` and `% (cached) (service worker)`, and lots more if extra ones > are added in the future. Happy to go either direction. That's +1 string compared to the current solution, I can live with that, especially if it means not having spaces and concatenations hidden in the code. I would question the UX choice of making a tooltip "xxx (cached) (service worker)", but that's a different story.
Attached image status_tooltip.png —
(In reply to Francesco Lodolo [:flod] from comment #7) > (In reply to Vangelis Katsikaros from comment #6) > > I thought about that. However, if I understand correctly, the strings are > > appended independently to the title, so it would take 2 localized strings > > `%S (cached)` and `% (cached) (service worker)`, and lots more if extra ones > > are added in the future. Happy to go either direction. > > That's +1 string compared to the current solution, I can live with that, > especially if it means not having spaces and concatenations hidden in the > code. > > I would question the UX choice of making a tooltip "xxx (cached) (service > worker)", but that's a different story. The latest patch pushed abuses? the L10N a bit and doesn't have any hardcoded whitespace. The drawback is that the tooltip will probably have a trailing space (see https://bugzilla.mozilla.org/attachment.cgi?id=8858775). If this is not desirable, I will proceed with the original suggestion.
Comment on attachment 8858742 [details] Bug 1192523 - "(cached)" tooltip of request method in netmonitor cannot be localized. https://reviewboard.mozilla.org/r/130776/#review133342 ::: devtools/client/locales/en-US/netmonitor.properties:813 (Diff revision 2) > # next to a header list item, with a link to external documentation > netmonitor.headers.learnMore=Learn More > > +# LOCALIZATION NOTE (netmonitor.status.tooltip): This is the tooltip > +# for the value of the column status code > +netmonitor.status.tooltip=%S %S %S %S As a general rule: when adding more than one variable, use %1$S, %2$S, etc. and explain in the comment how they are replaced. You could solve the trailing space problem by having 3 strings netmonitor.status.tooltip.cached = %1$S %2$S (cached) netmonitor.status.tooltip.worker = %1$S %2$S (service worker) netmonitor.status.tooltip.cachedworker = %1$S %2$S (cached, service worker) Unless I'm misunderstanding the current code, it would be enough, and a little more readable than "(cached) (service worker)". True, it won't scale, since a third option would mean adding 4 strings, but I'm not sure how likely that scenario is.
Comment on attachment 8858742 [details] Bug 1192523 - "(cached)" tooltip of request method in netmonitor cannot be localized. https://reviewboard.mozilla.org/r/130776/#review133344 ::: devtools/client/locales/en-US/netmonitor.properties:822 (Diff revision 3) > +# the column status code, when the request is from a service worker > +# %1$S is the status code, %2$S is the status text. > +netmonitor.status.tooltip.worker = %1$S %2$S (service worker) > + > +# LOCALIZATION NOTE (netmonitor.status.tooltip.cachedworker): This is the tooltip > +# of the column status code, when the request is cached and is from service worker Small nit: from *a* service worker And thanks, this looks much better from a localization point of view.
Comment on attachment 8858742 [details] Bug 1192523 - "(cached)" tooltip of request method in netmonitor cannot be localized. https://reviewboard.mozilla.org/r/130776/#review133344 > Small nit: from *a* service worker > > And thanks, this looks much better from a localization point of view. Thanks for all the input. Done.
Status: NEW → ASSIGNED
Comment on attachment 8858742 [details] Bug 1192523 - "(cached)" tooltip of request method in netmonitor cannot be localized. https://reviewboard.mozilla.org/r/130776/#review133714 Thanks for the patch! R+ assuming try is green. Honza
Attachment #8858742 - Flags: review?(odvarko) → review+
(In reply to Jan Honza Odvarko [:Honza] from comment #17) > Try push: > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=9e4fbda7b5840d9bba244c4852b73096047dc760 > > Honza Thanks for the try push, I fixed a bug I didn't notice before.
Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/autoland/rev/e104f6b7839c "(cached)" tooltip of request method in netmonitor cannot be localized. r=Honza
https://hg.mozilla.org/mozilla-central/rev/e104f6b7839c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: