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

Status: RESOLVED FIXED

(DevTools :: Shared Components, defect)

Description

[Nicolas Chevobbe [:nchevobbe]]

**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

---

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

This is not a React warning. It is issued from https://searchfox.org/mozilla-central/rev/61d400da1c692453c2dc2c1cf37b616ce13dea5b/dom/base/nsTreeSanitizer.cpp#1309

Comment 2

[Nicolas Chevobbe [:nchevobbe]]

Confirmed that it's not restricted to the new frontend

Comment 3

[Johann Hofmann [:johannh]]

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)...

Comment 4

[Nicolas Chevobbe [:nchevobbe]]

(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

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

My bad, it's tested in devtools/client/webconsole/test/browser_webconsole_bug_585991_autocomplete_popup.js

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1441885 - Remove warnings from autocomplete-popup.js; .

This was caused by an innerHTML assignment. Switching to replaceWith + cloneNode
does the trick.
The console test asserting this is passing.

Review commit: https://reviewboard.mozilla.org/r/226108/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/226108/

Comment 7

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8957170 [details]
Bug 1441885 - Remove warnings from autocomplete-popup.js; .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226108/diff/1-2/

Comment 8

[Julian Descottes [:jdescottes]]

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?

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8957170 [details]
Bug 1441885 - Remove warnings from autocomplete-popup.js; .

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/226108/diff/2-3/

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

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

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/354e4a21f310
Remove warnings from autocomplete-popup.js; r=jdescottes.

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/354e4a21f310