Web Inspector: Can't add a new class by editing class attribute in DOM outline

[Nikita Vasilyev]

Web Inspector automatically adds a non-breaking space when adding a CSS class (via double-click or enter).
Because this chains the class name, it removes any styles. Also, the ` ` isn't visible unless you're editing via "Edit as HTML."

Steps to Reproduce:
1. Open Web Inspector
2. Find an HTML element that has a value in the class attribute
3. Double-click the class attribute (or press Enter)
4. Add "hello" to the beginning of the attribute value
5. Right click and select "Edit as HTML"
6. Notice the ` `

Expected Results:
I could double-click / enter to quickly add a class and it wouldn't automatically add a non-breaking space.

Observed Results:
It adds a non-breaking space, making the quick edit functionality useless.

<rdar://problem/34144628>

[Nikita Vasilyev]

Created attachment 329534 [details]
[Image] Bug

[Nikita Vasilyev]

Devin, if you're looking for more bugs to fix, this's a good one! If not, feel free to set it back to unassigned.

[Nikita Vasilyev]

This happens only on certain class names.

Steps:
1. Open http://nv.github.io/webkit-inspector-bugs/styles-redesign/tests/color.html
2. Double-click on `class="item-1"`
3. Type "my-class " before the existing class name, so it looks like `class="my-class item-1"`
4. Press Enter

It looks like DOMAgent.setAttributesAsText *sometimes* converts spaces to  .

[Nikita Vasilyev]

(In reply to Nikita Vasilyev from comment #3)
> It looks like DOMAgent.setAttributesAsText *sometimes* converts spaces to
>  .

No, nbsp's are coming from contentEditable.

[Nikita Vasilyev]

Created attachment 335982 [details]
Patch

[Nikita Vasilyev]

"ContentEditable:   is forced on SPACE between text nodes"
https://bugs.chromium.org/p/chromium/issues/detail?id=310149
Fixed in Chromium on July 27, 2016 but appears to be unresolved in WebKit.
This makes we concerned about other places where we use contentEditable, such as the styles editor.

[Nikita Vasilyev]

(In reply to Nikita Vasilyev from comment #5)
> Created attachment 335982 [details]
> Patch

To clarify, this patch makes it impossible to commit non-breaking spaces in HTML attributes and I'm fine with that. When would you want to add a non-breaking space in Web Inspector? I can't think of any common cases.

[Nikita Vasilyev]

Created attachment 336078 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 336078 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=336078&action=review

> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:1069
> +        let nbspRegex = /\xA0/g;

Why not const?

[Matt Baker]

Comment on attachment 336078 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=336078&action=review

>> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:1069
>> +        let nbspRegex = /\xA0/g;
> 
> Why not const?

This should be const.

We haven't settled on a convention for applying const in the WebInspector frontend. I think more widespread usage would be a benefit, but my thoughts on const-correctness come from C++, and I don't know how much of that applies to JS.

[Nikita Vasilyev]

Created attachment 336107 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 336107 [details]
Patch

Clearing flags on attachment: 336107

Committed r229744: <https://trac.webkit.org/changeset/229744>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.