Closed Bug 1343980 Opened 3 years ago Closed 2 years ago

Text-overflow broken for table cell pseudo-elements

Categories

(Core :: Layout, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: bzbarsky, Assigned: mats)

Details

Attachments

(1 file)

Consider this testcase:

  <style>
    tr::before {
      content: "Some long text here that overflows and whatnot.";
      display: table-cell;
      overflow: hidden;
      white-space: nowrap;
      text-overflow: ellipsis;
    }  
  </style>
  <table style="table-layout: fixed; width: 130px">
    <tr></tr>
  </table>

This should show an ellipsis and does not.

The reason it does not is that the IsHorizontalOverflowVisible function in TextOverflow.cpp does this:

  nsIFrame* f = aFrame;
  while (f && f->StyleContext()->GetPseudo() &&
         f->GetType() != nsGkAtoms::scrollFrame) {
    f = f->GetParent();
  }

before checking for overflow styles.  In the example above, we get in here with aFrame being the ::-moz-cell-content of the table cell.  It has a pseudo, of course, so we step up to the cell.  That _also_ has a pseudo (::before), so we step up to the table row.  And the table row has visible overflow.  Note that there is no scrollframe in this case, so we never take that opportunity to break out of the loop.

If I change the testcase to set "overflow:hidden" on the table row, things work, which is bogus on its own.  Specifically, this testcase has broken rendering:

  <style>
    tr::before {
      content: "Some long text here that overflows and whatnot.";
      display: table-cell;
      white-space: nowrap;
      text-overflow: ellipsis;
    }  
    tr {
      overflow: hidden;
    }
  </style>
  <table style="table-layout: fixed; width: 130px">
    <tr></tr>
  </table>

in that it ellipsizes the text.  But if the "text-overflow" style were not there, it would just _show_ the text!

What is the goal of this GetPseudo() check?  Is it trying to step out of anonymous boxes?  Something else?
Flags: needinfo?(mats)
Yeah, I think I meant to check for anon frames here.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b518c83f175452f5a403c7c78810a9947953a88b
Assignee: nobody → mats
Flags: needinfo?(mats)
Attachment #8938822 - Flags: review?(bzbarsky)
Comment on attachment 8938822 [details] [diff] [review]
Skip anonymous boxes (rather than all pseudos) when looking for the 'overflow' style frame

I suspect bz is overloaded with reviews, can you take this one Daniel?
Attachment #8938822 - Flags: review?(bzbarsky) → review?(dholbert)
I'm not overloaded with reviews, but I _was_ on vacation until day before yesterday, so am still catching up on a few things.
Comment on attachment 8938822 [details] [diff] [review]
Skip anonymous boxes (rather than all pseudos) when looking for the 'overflow' style frame

r=me
Attachment #8938822 - Flags: review?(dholbert) → review+
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/58f33e7fefba
[css-ui] Skip anonymous boxes (rather than all pseudos) when looking for the 'overflow' style frame.  r=bz
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/58f33e7fefba
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.