747267 - very bad font inflation when filing a new bug

Status: RESOLVED WONTFIX

(Core :: Layout, defect, P1)

Description

[(no longer active)]

try filing a new bug. some of the description messages get inflatrd, but most of them are not. also the text on the search button is cropped to "Se".

Comment 1

[(no longer active)]

generally, font inflation is broken on bugzilla.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Did this get better or worse when bug 706193 landed?

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Or, alternately, if you're using a build within the past few days, does it get better or worse if you set the font.size.inflation.lineThreshold preference to 0?

Comment 4

[(no longer active)]

(In reply to David Baron [:dbaron] from comment #3)
> Or, alternately, if you're using a build within the past few days, does it
> get better or worse if you set the font.size.inflation.lineThreshold
> preference to 0?

That is better.  Now most of the text on the page is consistently inflated, except for links to the products.  The text on the search button is still cropped.

Comment 8

[Scott Johnson (:jwir3)]

See:
bug 748054, comment 3
bug 748054, comment 4

for additional info on this issue.

Comment 9

[Scott Johnson (:jwir3)]

From bug 748054, commnent 3:
> Fixing ycombinator (bug 707195) would require the patch series I've already written in 
> that bug, possibly plus patches to make overflow!=visible (i.e., nsHTMLScrollFrame) 
> not be an inflation flow root (which is what should fix reddit), 

This makes sense. I can start working on this.

> plus patches to make *some* table cells not be inflation flow
> roots (but leave others, otherwise we'd  regress nytimes.com).

This doesn't make sense to me... nytimes doesn't have tables in the layout that I can see? Are you saying that we'd regress the nytimes footer bug _in general_ if we make a change where all table cells were not flow roots?

> I think the underlying heuristic could perhaps be something 
> like whether there's inflatable text in more than one column of the table, but 
> figuring out the right way to do that and how to do it efficiently is likely to be 
> tricky.

So, this would work for Damon's case, but it's not going to work for the bugzilla case - bugzilla has (for the most part) a single table, with what appears to be at most 4 columns.

Comment 10

[Scott Johnson (:jwir3)]

What if we did something like this:

I propose adding a new nsIFrame pointer into nsTableFrame, called mLastFontContainerCell, which will only be used during reflow. (So we might be able to put it in the temporary reflow state that gets passed around). Then, what I would like to do is "bubble up" the font inflation container status from table cells to nsTableFrames IF a table frame contains two or more columns that are themselves font inflation containers. (Thus, two cells that would contain enough text to trigger font inflation by themselves).

Basically, if we have multiple tables nested within each other, on successive reflows, the tables would bubble up the font inflation status so that eventually, if there is enough text spread across all of the frames, the uppermost table would be the font inflation container/flow root.

During reflow of nsTableCellFrame:
----------------------------------

If (this frame is a font inflation container) {
  parentTable <- the parent table frame of this frame
  if (parentTable is a font inflation container) {
    remove the font inflation container and font inflation flow root
    state bits from this frame
  } else if (parentTable.mLastFontContainerCell != nsnull and
             column index of parentTable.mLastFontContainerCell != column index of this)        {
               remove the font inflation container and font inflation flow root 
               state bits from this frame.

               set font inflation container and flow root bits on parent frame**
             }
    else {
      parentTable.mLastFontContainerCell <- this
    }
}

** During this procedure, we'd probably have to "bubble up" the font inflation container status to the parent frame, if the table is the only child of its parent. Otherwise, the iterative procedure wouldn't work.

PROBLEMS:
---------
There are a couple of issues with this approach. Probably the most obvious is that it could regress situations where we don't want to inflate tables. I tried to get around this by only setting the table to be a flow root if we have one cell in at least two columns, that, by themselves would be inflated. I'm not sure how stable this will be, though.

Another problem is that if a table is nested within multiple divs, all of which are within another table, for example, like:

<table> 
...
<div id="div1">
...
<div id="div2">
<table>
 ...
</table>
...
</div>
</div>
</table>

then we still might run into inconsistent font inflation. This could happen if folks use tables for layout, which is still (I think) pretty common.

I guess I'm just looking at this as a potential starting place for this. Refinement is definitely necessary, and I'm not even sure this will work at all.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to Scott Johnson (:jwir3) from comment #9)
> This doesn't make sense to me... nytimes doesn't have tables in the layout
> that I can see? Are you saying that we'd regress the nytimes footer bug _in
> general_ if we make a change where all table cells were not flow roots?

There's a piece around the middle of the page that's in a table-layout:fixed table inside an overflow:hidden container -- it's the part that contains the horizontally-scrollable "INSIDE NYTIMES.COM" section that divides the two main parts of the page.

Comment 12

[Jet Villegas (inactive)]

Re-nomming this one to force a re-test against the builds posted for bug 707195.

Comment 13

[Johnathan Nightingale [:johnath]]

Discussed in triage: in the endgame, this is not a blocker

Comment 14

[Tony Chung [:tchung]]

qawanted: assigned to eric to retest bugzilla on latest nightly.

Comment 15

[Xiao Meng Wei :xwei]

Attachment: Font Inflation on SeaMonkey

As of today's nightly, the SeaMonkey section still seems to be inflated incorrectly.

Comment 16

[Paul Silaghi, QA [:pauly]]

Dropping qawanted based on comment 15.

Comment 17

[Brad Lassey [:blassey] (use needinfo?)]

David, what do you want to do with these old font inflation bugs?

Comment 18

[Brad Lassey [:blassey] (use needinfo?)]

assigning to dbaron

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm hoping to find a new owner for font inflation code at some point, but don't currently have a candidate.

Alternatively, I might have a chance to look into this at some point, but probably not in the next month or so.  (Heuristics are hard, and the basic problem we need to solve with them here is distinguishing tables whose cells are logically in a single flow of text from tables whose cells are logically separate flows of text, and doing this extremely efficiently in a way that doesn't require lots of updates during incremental layout resulting from addition of content.  Although it might help if we separated the decisions about disabling font inflation because the table is layout-ish from the decisions about what is a separate flow.)

Comment 20

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

seems fine on latest nightly (55).

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

We disabled the feature.