Bug 53438 - Text-overflow is broken for button elements
Summary: Text-overflow is broken for button elements
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Igor Trindade Oliveira
URL:
Keywords: HasReduction
: 100218 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-01-31 10:21 PST by Erik Arvidsson
Modified: 2022-06-02 23:10 PDT (History)
9 users (show)

See Also:


Attachments
Test case (231 bytes, text/html)
2011-01-31 10:21 PST, Erik Arvidsson
no flags Details
Patch (2.85 KB, patch)
2011-08-17 13:52 PDT, Igor Trindade Oliveira
no flags Details | Formatted Diff | Diff
Patch (5.59 KB, patch)
2011-08-17 14:45 PDT, Igor Trindade Oliveira
no flags Details | Formatted Diff | Diff
Patch (5.59 KB, patch)
2011-08-18 06:57 PDT, Igor Trindade Oliveira
no flags Details | Formatted Diff | Diff
Patch (5.65 KB, patch)
2011-08-18 07:06 PDT, Igor Trindade Oliveira
hyatt: review-
hyatt: commit-queue-
Details | Formatted Diff | Diff
Patch (7.56 KB, patch)
2011-08-22 14:40 PDT, Igor Trindade Oliveira
hyatt: review-
hyatt: commit-queue-
Details | Formatted Diff | Diff
Patch (7.56 KB, patch)
2011-08-22 15:02 PDT, Igor Trindade Oliveira
hyatt: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
rendering in safari, firefox, chrome (97.36 KB, image/png)
2022-06-02 20:33 PDT, Karl Dubost
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Erik Arvidsson 2011-01-31 10:21:51 PST
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.
Comment 1 Igor Trindade Oliveira 2011-08-17 13:52:10 PDT
Created attachment 104230 [details]
Patch

Proposed patch.
Comment 2 Erik Arvidsson 2011-08-17 13:54:44 PDT
Comment on attachment 104230 [details]
Patch

Missing tests?
Comment 3 Igor Trindade Oliveira 2011-08-17 14:45:07 PDT
Created attachment 104250 [details]
Patch

Updated patch. Add a simple test.
Comment 4 Igor Trindade Oliveira 2011-08-18 06:57:54 PDT
Created attachment 104333 [details]
Patch

Updated Patch. The expected result just makes sense for Qt.
Comment 5 WebKit Review Bot 2011-08-18 07:01:16 PDT
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.
Comment 6 Igor Trindade Oliveira 2011-08-18 07:06:10 PDT
Created attachment 104334 [details]
Patch

Updated patch. Fix style.
Comment 7 Dave Hyatt 2011-08-18 12:39:06 PDT
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.
Comment 8 Igor Trindade Oliveira 2011-08-22 14:40:50 PDT
Created attachment 104741 [details]
Patch

Proposed patch.
Comment 9 Dave Hyatt 2011-08-22 14:48:01 PDT
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.
Comment 10 Igor Trindade Oliveira 2011-08-22 14:58:27 PDT
(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.
Comment 11 Igor Trindade Oliveira 2011-08-22 15:02:16 PDT
Created attachment 104745 [details]
Patch

Updated patch.
Comment 12 Dave Hyatt 2011-08-22 15:22:30 PDT
Comment on attachment 104745 [details]
Patch

r=me
Comment 13 WebKit Review Bot 2011-08-22 16:51:45 PDT
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
Comment 14 WebKit Review Bot 2011-08-22 18:16:25 PDT
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
Comment 15 Andreas Kling 2011-08-23 10:30:11 PDT
Comment on attachment 104745 [details]
Patch

Let's try that once again.
Comment 16 WebKit Review Bot 2011-08-23 12:01:05 PDT
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
Comment 17 Igor Trindade Oliveira 2011-08-25 11:37:25 PDT
Manually committed r93799: http://trac.webkit.org/changeset/93799
Comment 18 Igor Trindade Oliveira 2011-08-25 14:02:39 PDT
Reopen the bug. Rollout of change set 93799 committed in change set http://trac.webkit.org/changeset/93820
Comment 19 Eric Seidel (no email) 2011-12-21 14:34:03 PST
Attachment 104745 [details] was posted by a committer and has review+, assigning to Igor Trindade Oliveira for commit.
Comment 20 Rob Buis 2013-02-20 13:19:07 PST
*** Bug 100218 has been marked as a duplicate of this bug. ***
Comment 21 Rob Buis 2013-02-20 13:44:29 PST
(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.
Comment 22 Igor Trindade Oliveira 2013-02-20 14:52:09 PST
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.
Comment 23 Karl Dubost 2022-05-23 14:45:22 PDT
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.
Comment 24 Karl Dubost 2022-06-02 20:33:47 PDT
Created attachment 459980 [details]
rendering in safari, firefox, chrome

Same rendering in all browsers.