Closed Bug 1431428 Opened 7 years ago Closed 7 years ago

Use DOM manipulations to insert add-on / toolkit icons in "controlling extension" message in the preferences

Categories

(Firefox :: Settings UI, enhancement, P5)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: Gijs, Assigned: Gijs)

References

Details

Attachments

(1 file)

We currently use innerHTML and it triggers an eslint warning. We can tidy this up a little.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Comment on attachment 8944468 [details] Bug 1431428 - use DOM instead of innerHTML for extension messaging in prefs, https://reviewboard.mozilla.org/r/214662/#review220288 Static analysis found 1 defect in this patch. - 1 defect found by mozlint You can run this analysis locally with: - `./mach lint check path/to/file` (Python/Javascript/wpt) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: browser/components/preferences/in-content/preferences.js:546 (Diff revision 1) > - let icon = url => `<image src="${url}" class="extension-controlled-icon"/>`; > + let icon = url => { > + let img = document.createElement("image"); > + img.src = url; > + img.className = "extension-controlled-icon"; > + return img; > + } Error: Missing semicolon. [eslint: semi]
(In reply to Code Review Bot [:reviewbot] from comment #4) > Error: Missing semicolon. [eslint: semi] Oops, fixed in the update.
Priority: -- → P5
Comment on attachment 8944468 [details] Bug 1431428 - use DOM instead of innerHTML for extension messaging in prefs, https://reviewboard.mozilla.org/r/214662/#review220386 ::: toolkit/modules/BrowserUtils.jsm:748 (Diff revision 2) > + getLocalizedFragment(doc, msg, ...nodesOrStrings) { > + // Ensure replacement points are indexed: > + for (let i = 1; i <= nodesOrStrings.length; i++) { > + if (!msg.includes("%" + i + "$S")) { > + msg = msg.replace(/%S/, "%" + i + "$S"); > + } > + } Can we log an error if `nodesOrStrings.length` is not equal to the number of insertion points in `msg`? Or throw an exception? I'll defer to your choice here but I think we should do something.
Attachment #8944468 - Flags: review?(jaws) → review+
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/2018c9901076 use DOM instead of innerHTML for extension messaging in prefs, r=jaws
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Depends on: 1432555
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: