Bug 108633 – No caret on empty contenteditable element with negative text-indent

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Jake Archibald]

Click on the green box, the caret may flash once in the correct place, but doesn't appear after that.

Once you start typing, the text appears, as does the caret.

Firefox & Opera show a caret, although in the wrong place.

[Alexey Proskuryakov]

See also: bug 20242, bug 58975, bug 108053. We probably have a few duplicates between these, and other bugs that I didn't find now.

[Radar WebKit Bug Importer]

<rdar://problem/13134969>

[Robert Hogan]

Created attachment 188942 [details]
Patch

[WebKit Review Bot]

Attachment 188942 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/repaint/paint-caret-in-div-with-negative-indent-expected.png', u'LayoutTests/fast/repaint/paint-caret-in-div-with-negative-indent-expected.txt', u'LayoutTests/fast/repaint/paint-caret-in-div-with-negative-indent.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/rendering/RenderBlock.cpp']" exit_code: 1
LayoutTests/fast/repaint/paint-caret-in-div-with-negative-indent-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ryosuke Niwa]

Comment on attachment 188942 [details]
Patch

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

> Source/WebCore/rendering/RenderBlock.cpp:1685
> +    bool caretBrowsing = frame()->settings() && frame()->settings()->caretBrowsingEnabled();
> +    if (textIndentOffset() < 0 && (caretBrowsing || (node() && node()->rendererIsEditable()))) {

Why do we want to check node()->rendererIsEditable() when caret browsing is disabled? rendererIsEditable() is a very expensive function, I'd rather not call it here.

[Robert Hogan]

(In reply to comment #5)
> (From update of attachment 188942 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=188942&action=review
> 
> > Source/WebCore/rendering/RenderBlock.cpp:1685
> > +    bool caretBrowsing = frame()->settings() && frame()->settings()->caretBrowsingEnabled();
> > +    if (textIndentOffset() < 0 && (caretBrowsing || (node() && node()->rendererIsEditable()))) {
> 
> Why do we want to check node()->rendererIsEditable() when caret browsing is disabled? rendererIsEditable() is a very expensive function, I'd rather not call it here.

rendererIsEditable() only gets expensive if you need to climb the tree looking for a HTML element. I don't think that will happen very often here, especially where a negative text indent has been defined. Does that sound reasonable?

It might be cheaper overall to just add the overflow for any element that has the text indent rather than ones that will want to display a caret at some point. But I don't think rendererIsEditable() ever gets expensive here.

I need to remove the multiple calls to textIndentOffset().

[Robert Hogan]

Created attachment 189126 [details]
Patch

[WebKit Review Bot]

Attachment 189126 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/repaint/paint-caret-in-div-with-negative-indent-expected.png', u'LayoutTests/fast/repaint/paint-caret-in-div-with-negative-indent-expected.txt', u'LayoutTests/fast/repaint/paint-caret-in-div-with-negative-indent.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/rendering/RenderBlock.cpp']" exit_code: 1
LayoutTests/fast/repaint/paint-caret-in-div-with-negative-indent-expected.png:0:  Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png").  [image/png] [5]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Comment on attachment 189126 [details]
Patch

Clearing flags on attachment: 189126

Committed r143483: <http://trac.webkit.org/changeset/143483>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

This caused a bad repaint regression, when authors combine negative text-indent with overflow:hidden (which is what we recommend they do): bug 121137.

[Alexey Proskuryakov]

This bug still appears fixed after rollout (maybe <http://trac.webkit.org/changeset/143313> also took care of the same?)

Please comment and possibly open a new bug if the next nightly doesn't quite work for you in this regard.