Closed Bug 1508985 Opened Last year Closed 10 months ago

Enable ESLint for dom/tests/browser/

Categories

(Core :: DOM: Core & HTML, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox65 --- wontfix
firefox66 --- fixed

People

(Reporter: standard8, Assigned: VinceDaDev)

References

(Blocks 1 open bug)

Details

(Whiteboard: [seneca-eslint])

Attachments

(2 files, 1 obsolete file)

As part of rolling out ESLint across the tree, we should enable it for dom/tests/browser/.
Priority: -- → P3
I would like to be assigned to this issue. Thanks!
Assignee: standard8 → VinceDaDev
Hi,
    I'm just finishing up my manual changes and I'm stuck on a few identical errors regarding "unsafe assignment to innerHTML no-unsanitized/property" in both the "dom/tests/browser/helper_largeAllocation.js"(line 106) and "dom/test/browser/position.html"(line 8 and 12). Can you give me some advice on fixing this situation?
(In reply to Vincent Wong from comment #2)
> Hi,
>     I'm just finishing up my manual changes and I'm stuck on a few identical
> errors regarding "unsafe assignment to innerHTML no-unsanitized/property" in
> both the "dom/tests/browser/helper_largeAllocation.js"(line 106) and
> "dom/test/browser/position.html"(line 8 and 12). Can you give me some advice
> on fixing this situation?

As we're in tests, the rule isn't as important as other code. For now, we'll take the easier route and just disable the rule for the specific cases.

So for example you can just add this comment on the line before:

// eslint-disable-next-line no-unsanitized/property
await ContentTask.spawn(aBrowser, TEST_URI, TEST_URI => {
  content.document.body.innerHTML = `<iframe src='${TEST_URI}'></iframe>`;
Depends on D14017

Hi,

I have just updated the changes and submitted them but I can't seem to find the changes I've made on here.
I also tried doing another hg commit -m "..." again but it gave me "nothing changed".

Hopefully you see the changes. If you don't see it then let me know. It might be because I used hg histedit and merged the updated commit with my (manual changes).

Hi Vincent, thank you for the update.

You can see the changes here: https://phabricator.services.mozilla.com/D14018?vs=42792&id=49016&whitespace=ignore-most#toc

(if you select the "History" tab of the table, you can select which versions to diff).

I'll check them in the morning, but a quick glance they look good.

Depends on D14018

Attachment #9037403 - Attachment is obsolete: true
Attachment #9030108 - Attachment description: Bug 1508985 - Enable EsLint for dom/tests/browser/ (automatic changes) → Bug 1508985 - Enable ESLint for dom/tests/browser/ (automatic changes)
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/efd90c73657e
Enable ESLint for dom/tests/browser/ (automatic changes) r=Standard8,qdot
https://hg.mozilla.org/integration/autoland/rev/62947b5b105b
Enable ESLint for dom/tests/browser (manual changes) r=Standard8,qdot

Hi Vince,

Thank you for all your work on this. This has now landed on our integration branch. Assuming no issues are found, it'll be merged to our main tree within 24 hours at which time the bug will be marked as fixed.

Thank you again, and for sticking with it.

Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.