Closed Bug 1354647 Opened 3 years ago Closed 3 years ago
The eyedropper highlighter needs a l10n string that is actually part of the /devtools/client folder
59 bytes, text/x-review-board-request
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
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 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.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/15538e73c15e Move the eyedropper l10n strings to /devtools/shared/; r=jryans
You need to log in before you can comment on or make changes to this bug.