Bug 210197 – REGRESSION (r251254): Web Inspector: Text insertion point is invisible when editing DOM nodes

• Component: Web Inspector
• Status: RESOLVED FIXED

[Michael Bailey]

Created attachment 395828 [details]
Video showing bug

Sometime between Safari 13.0 and 13.1, the blinking text insertion point cursor became invisible when editing DOM nodes. See attached video for an example. The insertion point is still visible as expected when editing style rules, in the console and other areas of the Inspector—it's just when editing DOM nodes that it's invisible.

This issue reproduces in the shipping Safari 13.1 (15609.1.20.111.8) and Safari Technology Preview 104 (Safari 13.2, WebKit 15610.1.8.3).

[Radar WebKit Bug Importer]

<rdar://problem/61485409>

[Nikita Vasilyev]

Bisecting shows that this regressed in Bug 203057 - Web Inspector: Elements: selection shouldn't be dimmed by shadow trees.

[Nikita Vasilyev]

Created attachment 402025 [details]
[Image] Memory issue

Another issue with https://trac.webkit.org/changeset/251254 is that it introduced a layer for every DOM node.

[Nikita Vasilyev]

Created attachment 402029 [details]
Patch

[Nikita Vasilyev]

Created attachment 402030 [details]
[Video] With patch applied

[Devin Rousso]

Comment on attachment 402029 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.css:55
> +    z-index: -1;

This reintroduces the bug that r251254 solved, which is that the selection background is shaded by the semi-transparent shadow DOM background, which is not ideal (especially when selecting something inside multiple nested shadow DOM subtrees).

Why is it that adding `z-index: 1;` causes the cursor to disappear?  That seems like the real bug here 🤔

[Nikita Vasilyev]

(In reply to Devin Rousso from comment #6)
> Comment on attachment 402029 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=402029&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.css:55
> > +    z-index: -1;
> 
> This reintroduces the bug that r251254 solved, which is that the selection
> background is shaded by the semi-transparent shadow DOM background, which is
> not ideal (especially when selecting something inside multiple nested shadow
> DOM subtrees).

To a smaller extend, but yes.

> 
> Why is it that adding `z-index: 1;` causes the cursor to disappear?  That
> seems like the real bug here 🤔

This seems like a contentEditable WebKit bug although I haven't been able to make a reduction yet.

[Nikita Vasilyev]

Created attachment 402156 [details]
[Image] Before/after

Actually, I wouldn't say it re-introduced the bug that r251254 solved. Screenshot attached. The text to background contrast of the selected item is pretty good!

[Devin Rousso]

(In reply to Nikita Vasilyev from comment #8)
> Created attachment 402156 [details]
> [Image] Before/after
> 
> Actually, I wouldn't say it re-introduced the bug that r251254 solved. Screenshot attached. The text to background contrast of the selected item is pretty good!

I agree that it looks better than it did before r251254, but r251254 was specifically about "selection shouldn't be dimmed by shadow trees" which is still happening with this patch and is something I think we should avoid if possible.

[Nikita Vasilyev]

OK, this will require more changes then.

[Nikita Vasilyev]

Created attachment 407788 [details]
Patch

Updated the Changelog and rebaselined.

I think we should land this now. When "Bug 213501 - contentEditable: Text caret is invisible on position relative elements" is resolved, we should revert this change (if the memory issue in Comment 3 isn't actually an issue).

[Devin Rousso]

Comment on attachment 407788 [details]
Patch

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

r=me

Please make a new bug for fixing the non-solid issue that's blocked by bug 213501.

> Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.css:47
> +    --shadow-background-color: hsla(0, 0%, 0%, 0.05);

this could use a better name, like `--shadow-subtree-background-color`

> Source/WebInspectorUI/UserInterface/Views/DOMTreeOutline.css:-77
> -.tree-outline.dom li > :not(.selection-area) {

(note for the future) we probably only need to do this when inside a `.shadow`

[Nikita Vasilyev]

Created attachment 407800 [details]
Patch

[EWS]

Committed r266499: <https://trac.webkit.org/changeset/266499>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407800 [details].

[Nikita Vasilyev]

*** Bug 208310 has been marked as a duplicate of this bug. ***