The eyedropper highlighter needs a l10n string that is actually part of the /devtools/client folder

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Developer Tools: Inspector
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: pbro, Assigned: pbro)

Tracking

unspecified
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Fennec only get what's inside /devtools/server/ and /devtools/shared, so code inside this folder should never need to import code that's inside /devtools/client.

I found one instance where we didn't respect this rule:

In file /devtools/server/actors/highlighters/eye-dropper.js

we do:

loader.lazyGetter(this, "l10n",
  () => Services.strings.createBundle("chrome://devtools/locale/eyedropper.properties"));

Knowing that the eyedropper.properties file is actually here:

devtools\client\locales\en-US\eyedropper.properties

I know why this is the case: the eyedropper used to be a client-side module. And I moved it to the server in bug 1262439, but I didn't think about moving the strings.

So let's do this here.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
To be clear: this won't be a problem on desktop, but will most probably make the eyedropper fail on Fennec. However, the eyedropper has probably never been used on Fennec, which is why a bug was never reported.

Comment 3

11 months ago
mozreview-review
Comment on attachment 8855923 [details]
Bug 1354647 - Move the eyedropper l10n strings to /devtools/shared/;

https://reviewboard.mozilla.org/r/127808/#review130498

Thanks!
Attachment #8855923 - Flags: review?(jryans) → review+
Thanks Ryan.
I tested this locally and the strings seem to load properly, so I don't think a try build is necessary, but just in case, here's one: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b578b723978f4b63ae36108a221853d4c03a30ce
Let's just wait for a few tests to pass and then I'll kill the rest of them and land the commit.

Comment 5

11 months ago
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/15538e73c15e
Move the eyedropper l10n strings to /devtools/shared/; r=jryans
https://hg.mozilla.org/mozilla-central/rev/15538e73c15e
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.