Bug 53438 – Text-overflow is broken for button elements

• Component: CSS
• Status: RESOLVED WORKSFORME

[Erik Arvidsson]

Created attachment 80654 [details]
Test case

Button elements ignore the text-overflow CSS property. This is probably related to bug 5990 since buttons use a flex box shadow DOM.

[Igor Trindade Oliveira]

Created attachment 104230 [details]
Patch

Proposed patch.

[Erik Arvidsson]

Comment on attachment 104230 [details]
Patch

Missing tests?

[Igor Trindade Oliveira]

Created attachment 104250 [details]
Patch

Updated patch. Add a simple test.

[Igor Trindade Oliveira]

Created attachment 104333 [details]
Patch

Updated Patch. The expected result just makes sense for Qt.

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

LayoutTests/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 6 files


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

[Igor Trindade Oliveira]

Created attachment 104334 [details]
Patch

Updated patch. Fix style.

[Dave Hyatt]

Comment on attachment 104334 [details]
Patch

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

r-

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:1222
> -    bool hasTextOverflow = style()->textOverflow() && hasOverflowClip();
> +    bool hasTextOverflow = style()->textOverflow();

I think you probably wanted this to be:

bool hasTextOverflow = style()->textOverflow() && (hasOverflowClip() || hasControlClip());

It's definitely incorrect to make text overflow apply when overflow is visible. I suspect this is just a case where the control clip needed to be considered as well since it's supposed to behave like overflow in this case.

[Igor Trindade Oliveira]

Created attachment 104741 [details]
Patch

Proposed patch.

[Dave Hyatt]

Comment on attachment 104741 [details]
Patch

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

r-

> Source/WebCore/rendering/RenderBox.h:346
> +    virtual bool hasControlClip() const { return parentBox() && parentBox()->hasControlClip() && isAnonymousBlock(); }

I'd move isAnonymousBlock to be the first condition in the return statement instead of the last.

> Source/WebCore/rendering/RenderBox.h:347
> +    virtual LayoutRect controlClipRect(const IntPoint&) const;

Don't change the interface here. Should still be layoutpoint.

[Igor Trindade Oliveira]

(In reply to comment #9)
> (From update of attachment 104741 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=104741&action=review
> 
> r-
> 
> > Source/WebCore/rendering/RenderBox.h:346
> > +    virtual bool hasControlClip() const { return parentBox() && parentBox()->hasControlClip() && isAnonymousBlock(); }
> 
> I'd move isAnonymousBlock to be the first condition in the return statement instead of the last.
> 

Ok

> > Source/WebCore/rendering/RenderBox.h:347
> > +    virtual LayoutRect controlClipRect(const IntPoint&) const;
> 
> Don't change the interface here. Should still be layoutpoint.

Sorry, it was the leftover of the last patch.

[Igor Trindade Oliveira]

Created attachment 104745 [details]
Patch

Updated patch.

[Dave Hyatt]

Comment on attachment 104745 [details]
Patch

r=me

[WebKit Review Bot]

Comment on attachment 104745 [details]
Patch

Rejecting attachment 104745 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 2

Last 500 characters of output:
  fast/text/international/danda-space.html = IMAGE
  fast/text/international/thai-baht-space.html = IMAGE
  fast/text/international/thai-line-breaks.html = IMAGE
  platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE
  svg/custom/inline-svg-in-xhtml.xml = IMAGE
  tables/mozilla/bugs/bug138725.html = IMAGE
  tables/mozilla_expected_failures/collapsing_borders/bug41262-5.html = IMAGE
  tables/mozilla_expected_failures/collapsing_borders/bug41262-6.html = IMAGE



Full output: http://queues.webkit.org/results/9478015

[WebKit Review Bot]

Comment on attachment 104745 [details]
Patch

Attachment 104745 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9468964

New failing tests:
svg/custom/inline-svg-in-xhtml.xml
fast/forms/control-clip-overflow.html
tables/mozilla_expected_failures/collapsing_borders/bug41262-5.html
tables/mozilla_expected_failures/collapsing_borders/bug41262-6.html
tables/mozilla/bugs/bug138725.html

[Andreas Kling]

Comment on attachment 104745 [details]
Patch

Let's try that once again.

[WebKit Review Bot]

Comment on attachment 104745 [details]
Patch

Rejecting attachment 104745 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-03', '--port..." exit_code: 2

Last 500 characters of output:
  fast/text/international/danda-space.html = IMAGE
  fast/text/international/thai-baht-space.html = IMAGE
  fast/text/international/thai-line-breaks.html = IMAGE
  platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE
  svg/custom/inline-svg-in-xhtml.xml = IMAGE
  tables/mozilla/bugs/bug138725.html = IMAGE
  tables/mozilla_expected_failures/collapsing_borders/bug41262-5.html = IMAGE
  tables/mozilla_expected_failures/collapsing_borders/bug41262-6.html = IMAGE



Full output: http://queues.webkit.org/results/9478487

[Igor Trindade Oliveira]

Manually committed r93799: http://trac.webkit.org/changeset/93799

[Igor Trindade Oliveira]

Reopen the bug. Rollout of change set 93799 committed in change set http://trac.webkit.org/changeset/93820

[Eric Seidel (no email)]

Attachment 104745 [details] was posted by a committer and has review+, assigning to Igor Trindade Oliveira for commit.

[Rob Buis]

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

[Rob Buis]

(In reply to comment #19)
> Attachment 104745 [details] was posted by a committer and has review+, assigning to Igor Trindade Oliveira for commit.

I am confused, is this ready to go in or are the reported fails real?
Cheers,

Rob.

[Igor Trindade Oliveira]

The fails are real. 

(In reply to comment #21)
> (In reply to comment #19)
> > Attachment 104745 [details] [details] was posted by a committer and has review+, assigning to Igor Trindade Oliveira for commit.
> 
> I am confused, is this ready to go in or are the reported fails real?
> Cheers,
> 
> Rob.

[Karl Dubost]

I'm tempted to close here as Safari and Firefox latest versions behave as expected (aka ellipsis is visible). This has been fixed elsewhere probably.

[Karl Dubost]

Created attachment 459980 [details]
rendering in safari, firefox, chrome

Same rendering in all browsers.