"Removed unsafe attribute" warning in autocomplete-popup.js

RESOLVED FIXED in Firefox 60

Status

defect
RESOLVED FIXED
a year ago
10 months ago

People

(Reporter: nchevobbe, Assigned: nchevobbe)

Tracking

(Blocks 1 bug)

Trunk
Firefox 60
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(1 attachment)

**Steps to reproduce**
1. Open the browser console
2. Open the console
3. Type `a` in the console input


**Actual results**

There's a React warning in the browser console: 

Removed unsafe attribute. Element: li. Attribute: xmlns. 5 autocomplete-popup.js:392:4

---
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
This is not a React warning. It is issued from https://searchfox.org/mozilla-central/rev/61d400da1c692453c2dc2c1cf37b616ce13dea5b/dom/base/nsTreeSanitizer.cpp#1309
Summary: React warning in autocomplete-popup.js → "Removed unsafe attribute" warning in autocomplete-popup.js
Confirmed that it's not restricted to the new frontend
Assignee: nchevobbe → nobody
Status: ASSIGNED → NEW

Updated

a year ago
Blocks: 1437147
It's because of this innerHTML assignment, I guess: https://searchfox.org/mozilla-central/rev/33cc9e0331da8d9ff3750f1e68d72d61201176cb/devtools/client/shared/autocomplete-popup.js#392

That looks pretty unnecessary to me, if I'm not mistaken. We should convert it to use cloneNode(true)...
Blocks: 1396738
(In reply to Johann Hofmann [:johannh] from comment #3)
> It's because of this innerHTML assignment, I guess:
> https://searchfox.org/mozilla-central/rev/
> 33cc9e0331da8d9ff3750f1e68d72d61201176cb/devtools/client/shared/autocomplete-
> popup.js#392
> 
> That looks pretty unnecessary to me, if I'm not mistaken. We should convert
> it to use cloneNode(true)...

Right. Using cloneNode removed the warnings.
Sadly I think this is not tested, so we need to make sure the change doesn't break accessibility
My bad, it's tested in devtools/client/webconsole/test/browser_webconsole_bug_585991_autocomplete_popup.js
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 8

a year ago
mozreview-review
Comment on attachment 8957170 [details]
Bug 1441885 - Remove warnings from autocomplete-popup.js; .

https://reviewboard.mozilla.org/r/226108/#review232152

Thanks for the fix Nicolas!

::: devtools/client/shared/autocomplete-popup.js:69
(Diff revision 2)
>    this._list = this._document.createElementNS(HTML_NS, "ul");
>    this._list.setAttribute("flex", "1");
>  
> -  // The list clone will be inserted in the same document as the anchor, and will receive
> -  // a copy of the main list innerHTML to allow screen readers to access the list.
> -  this._listClone = this._document.createElementNS(HTML_NS, "ul");
> +  // The list clone will be inserted in the same document as the anchor, and will be a
> +  // copy of the main list to allow screen readers to access the list.
> +  this._listClone = this._list.cloneNode();

Could we assign the appropriate classname here as well?
Attachment #8957170 - Flags: review?(jdescottes) → review+
Comment hidden (mozreview-request)
(Assignee)

Comment 10

a year ago
mozreview-review-reply
Comment on attachment 8957170 [details]
Bug 1441885 - Remove warnings from autocomplete-popup.js; .

https://reviewboard.mozilla.org/r/226108/#review232152

> Could we assign the appropriate classname here as well?

Sure. Thanks for the review :)

Comment 11

a year ago
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/354e4a21f310
Remove warnings from autocomplete-popup.js; r=jdescottes.

Comment 12

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/354e4a21f310
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60

Updated

10 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.