On comments on news.ycombinator.com, comments get inflated to different sizes depending on their nesting depth, resulting in bizarre output. Hacker News uses nested <table> elements to render comments.
This might just well be expected behavior for sites like this. I think iOS Safari may well do the same thing here. Or does it somehow manage to do better? (We could do better if we could determine which blocks were part of the same flow and which weren't, but I don't know any decent heuristic for that.)
I just looked at this on an iPhone, and while it happens some there, it's not nearly as bad. Fewer comments are shrunk, and they're shrunk by less.
This screenshot is on an HTC evo 4g, with Nightly from 12-7-11.
(In reply to David Baron [:dbaron] from comment #2) > This might just well be expected behavior for sites like this. I think iOS > Safari may well do the same thing here. Or does it somehow manage to do > better? (We could do better if we could determine which blocks were part of > the same flow and which weren't, but I don't know any decent heuristic for > that.) Hmm.. well, could we take the outermost <table> element and somehow scale the font size to the largest available of all its children? So, for example, if we had: <table> <tr> <td> Some text 1 </td> <td> <table> <tr> <td>Some text 2. </td> </tr> </table> </td> </tr> </table> If we found an appropriate size to inflate the "some text 2" text at, could we bound "some text 1" by that, so that it can inflate to no greater than that size? It would require a two-pass solution, though, since we'd have to figure out the font inflation size of all of a frame's children before we could set the font inflation size for that frame.
The problem is that it's hard to distinguish that case from the (probably more common) case where the outer table doesn't have any text in it and the inner table does have text in it. In that case, we want the inflation to be based on the inner table.
Isn't that the case that Scott was talking about? That is, he proposed the text in the outer table (Some text 1) be bounded (from above) by the text in the inner table (Some text 2.), if I'm reading his comment right.
Ah, I should clarify, I mean that regardless of whether there's any text in the outer table, what he proposed would not impose any (additional) limits on text in the inner table.
Whiteboard: [font-inflation: nested tables] → [font-inflation: nested tables][readability]
Priority: P2 → P1
I thought I had a plan for fixing this in bug 706193 comment 13, but I don't think it's going to work. I think this should not be a beta blocker, and should perhaps even be wontfix.
Actually, I think I can make it work by only considering the widths of things that are ancestors of the first piece of text -- I think that should be sufficient, and we know those.
Reassigning to dbaron, since he's attacking this in combination with bug 706193.
Assignee: sjohnson → dbaron
dbaron, blassey asked for status on this bug during the mobile engineering meeting today. I said you told me that this likely won't be worth fixing, but I just wanted to ping you for an update in the bug, in the event that something has changed since last we spoke.
This was previously blocking-fennec1.0: beta+. I'm renominating with the suggestion that we punt on this bug for now and minus it. I tried to fix this as part of work on bug 706193, but I couldn't find a reasonable way around the basic chicken-and-egg problem: to fix this we'd want to know the widths of the containers of all the text, which requires doing layout on the whole subtree (or at least a decent part of it), but we need to know the inflation numbers before we do layout. (It's probably doable by constructing reflow states for the entire subtree as we walk it, but that would be a significant performance hit that I don't think we want to take.) For the record, the point where I gave up was https://hg.mozilla.org/users/dbaron_mozilla.com/patches/rev/3b2eb3585f6a I'd also note that at least based on my memory, I think iOS Safari has similar behavior on this site, though comment 3 notes that our differentiation is more drastic.
blocking-fennec1.0: beta+ → ?
(In reply to David Baron [:dbaron] from comment #14) > This was previously blocking-fennec1.0: beta+. I'm renominating with the > suggestion that we punt on this bug for now and minus it. > > I'd also note that at least based on my memory, I think iOS Safari has > similar behavior on this site, though comment 3 notes that our > differentiation is more drastic. I read HN a lot on my phone, and this bug makes it a pretty poor experience. Somewhere between 1 in 5 and 1 in 10 comments are unreadable. The iOS Safari experience is very different -- you can tell there's a bug, but every comment is readable.
dbaron, we talked through a strategy for fixing this a few weeks ago. What has changed since then?
blocking-fennec1.0: ? → +
I tried doing it, and couldn't figure out how (see comment 14). (Once display list based invalidation lands, we can probably make some architectural changes to Reflow that would make this easier, though.)
I see very similar behavior on the headlines on reddit.com. (Also see http://reddit.com/r/TrueReddit) Desktop and Opera Mobile both display it correctly.
An easier way to fix this just occurred to me, I think: just use the ncaWidth that we compute in the nsFontInflationData for the width (and thus mostly get rid of the idea of a font inflation container).
Actually, that would likely fix reddit, but it wouldn't fix ycombinator since ycombinator is using a separate table for each comment.
Can we do the reddit fix and then continue on with ycombinator as a separate bug?
I have a set of patches, but it doesn't fix reddit either. Not sure where to put it. It's possible it might be useful as progress towards a fix, but I'm skeptical. (Need to debug reddit a bit, but for some reason the inspector devtool mysteriously stopped working for me, so I need to figure out how to get it working again to do so.) There's a danger to trying to tweak this continuously. What font inflation is is a set of heuristics to (a) figure out what text forms a coherent chunk and ought to be treated together and (b) to figure out which pieces of text are safe to be inflated without breaking the page. Any changes to these heuristics to fix one page have a risk of breaking another page, and they (being heuristics) are never going to be perfect. At some point we have to stop tweaking and either decide that we want the feature as a whole or we don't want it; I'd like to know when that's going to be. (If we don't want it, an alternative is implementing Android-browser-style zooming that reflows on rewrap.) I'd also note that (from memory, it's been a little while since I've done this testing, but perhaps somebody could verify) other mobile browsers don't do particularly well on this sort of page either. My memory is that, on news.ycombinator.com comment pages: * iOS Safari (which also uses a font inflation approach) limits its inflation and thus preserves the visual appearance better but also doesn't actually inflate the text enough so that it's readable, which thus means that scrolling side-to-side for each line is needed (at least with the device in portait mode) * Opera mobile does a bit better, but still requires (as we do) resetting of context for reading each comment (not as bad as for each line), though in Opera mobile's case the context the user has to change is only horizontal panning rather than panning and zooming * Android native browser (on my Android devices, anyway) does nothing at all to make the text readable, and it requires scrolling side-to-side for each line. (On some Android versions or devices the Android native browser takes a zooming approach like what I see on Opera mobile, but I haven't actually seen that in action for ages, since I don't have such a device/version.)
(The reason we don't inflate reddit at all, by the way, is that we detect it as a mobile site because of its <meta name="viewport" content="width=1024" />, which leads to nsContentUtils::GetViewportInfo returning a value that has defaultZoom set to 1.0, which in turn means that nsLayoutUtils::FontSizeInflationEnabled returns false.)
And the reason the patches wouldn't fix reddit even without the <meta viewport> is reddit's use of overflow:hidden/overflow:auto in a bunch of places, which creates an inflation flow root. Now, I think I could change things so that overflow:hidden / overflow:auto don't make something an inflation flow root, except I remember dealing with that case when working on bug 706193, so I think either (a) there was some technical issue that made that necessary to avoid crashing or (b) one of the sites that was fixed by bug 706193 depended on making overflow!=visible make something an inflation flow root. For the record, the patches I'm testing are: https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/a54e29a0bbb6/save-nca-width https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/a54e29a0bbb6/change-to-auto-disable-for-shrink-wrap https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/a54e29a0bbb6/use-constant-width https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/a54e29a0bbb6/fix-list-control-frame https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/a54e29a0bbb6/remove-width-determination https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/a54e29a0bbb6/remove-current-inflation-container and they might be a useful simplification anyway.
Madhava, Ian: We're interested in your thoughts on this issue.
From a UX perspective, we should really try to fix this somehow. Reddit and ycombinator look terrible in our browser right now. Some examples: ycombinator http://cl.ly/0l2f2v0p3v0e290J0A0W Reddit http://cl.ly/1M452m2a2D3D04070S1I These pages look perfectly fine in stock Browser -- they are a little wonky in Chrome but still better than us.
Duplicate of this bug: 748026
Depends on: 752317
Depends on: 752029
No longer depends on: 752029
Depends on: 752033
(In reply to David Baron [:dbaron] from comment #23) > I'd also note that (from memory, it's been a little while since I've done > this testing, but perhaps somebody could verify) other mobile browsers don't > do particularly well on this sort of page either. So, from this post I assume that text inflation is not standardised? Maybe getting some input from other vendors would be a good idea before this turns into another endless source of frustration for designers.
(In reply to Tim from comment #30) > So, from this post I assume that text inflation is not standardised? Maybe > getting some input from other vendors would be a good idea before this turns > into another endless source of frustration for designers. I've started trying to do this; see http://dev.w3.org/csswg/css-size-adjust/ . However, so far Apple hasn't provided feedback. (And I also haven't had a chance to update it to changes I've made recently.)
(In reply to David Baron [:dbaron] from comment #25) > And the reason the patches wouldn't fix reddit even without the <meta > viewport> is reddit's use of overflow:hidden/overflow:auto in a bunch of > places, which creates an inflation flow root. Now, I think I could change > things so that overflow:hidden / overflow:auto don't make something an > inflation flow root, except I remember dealing with that case when working > on bug 706193, so I think either (a) there was some technical issue that > made that necessary to avoid crashing or (b) one of the sites that was fixed > by bug 706193 depended on making overflow!=visible make something an > inflation flow root. It turns out to be (b) -- removing this breaks http://www.nytimes.com/ again (the original problem in bug 706193), though probably most of the other things fixed by bug 706193 are still fixed.
(In reply to David Baron [:dbaron] from comment #32) > (In reply to David Baron [:dbaron] from comment #25) > > And the reason the patches wouldn't fix reddit even without the <meta > > viewport> is reddit's use of overflow:hidden/overflow:auto in a bunch of > > places, which creates an inflation flow root. Now, I think I could change > > things so that overflow:hidden / overflow:auto don't make something an > > inflation flow root, except I remember dealing with that case when working > > on bug 706193, so I think either (a) there was some technical issue that > > made that necessary to avoid crashing or (b) one of the sites that was fixed > > by bug 706193 depended on making overflow!=visible make something an > > inflation flow root. > > It turns out to be (b) -- removing this breaks http://www.nytimes.com/ again > (the original problem in bug 706193), though probably most of the other > things fixed by bug 706193 are still fixed. Er, oops, I was testing in a build with a non-default lineThreshold pref. It actually seems fine.
I'm going to put the first phase of work for this bug in bug 747720, since that's the first dependency I found that's entirely fixed by that first phase. (I think reddit also would be, if it weren't for bug 756518.)
So, the basic thing with tables is that we want: (a) data tables *not* to make inflation flow roots (b) layout tables used for indentation *not* to make inflation flow roots (required to fix this bug) (c) layout tables used as grids to make their cells inflation flow roots (required not to break http://www.nytimes.com/) The question is how to find a way to distinguish case (c) from cases (a) and (b) that: (1) is accurate enough (2) doesn't involve too many heuristics that make it hard for authors to understand the behavior (3) doesn't cause too many cases where a tiny change to the page can cause the result to change I'm still thinking about whether it's better to start from the assumption that we do want to make cells flow roots and look for cases where we shouldn't, or the other way around.
So I have a patch that fixes this while simultaneously not breaking bug 706193. However, it uses a complicated condition that feels to me like it's just fixing a single site, and I think it's a bad idea to complicate the Web platform to fix a single site rather than to fix a real pattern. I've made a set of three test builds to allow others to test this. They vary in when they consider table cells to be a barrier for scoping the region in which we make font size inflation decisions. The first build is equivalent to a mozilla-central build from a few days ago (in particular, based on https://hg.mozilla.org/mozilla-central/rev/8cf563a575fd) and always considers table cells to be a barrier: http://firstname.lastname@example.org/ The second build uses the complicated condition that fixes this bug while not breaking bug 706193: http://email@example.com/ The third build changes to *never* considers table cells to be a barrier, which fixes this bug *and* breaks bug 706193: http://firstname.lastname@example.org/ So what I'd like to know is: (a) where do the first and third builds produce different behavior? (So far, the only two sites I know of are ycombinator.com comments and the nytimes.com homepage) (b) in each of those cases, which of the first and third build is better, and does the second build go with the better option? Obvious places to look for things to test are dependencies/blockers of this bug, bug 706193, and bug 747720, but perhaps any other readibility bugs.
And I'd note that it may be easier to test using the desktop builds by setting the font.size.inflation.emPerLine preference to some number in the range 10-20 than by typing lots of urls on the Android builds.
And one further note: any bug where builds one and three don't make a difference should not be a dependency of this bug.
Re-nomming this one to force a re-test against the posted builds.
blocking-fennec1.0: + → ?
Summary: Text inflation inflates comments on news.ycombinator.com to different sizes → Text inflation inflates comments on news.ycombinator.com to different sizes (because inflation computed separately for different table cells)
Build 1 Bugzilla: font inflation issue is noticeable for the text on the Search button and for the text above the Search input field. Rotating the device to landscape will not resolve the issue and it still persist if the device is switched back to portrait. Google News: the main news frame covers the entire screen when the page is loaded for the 1st time and if the page is panned, bug 747720 is easy noticeable for some news entries. The left and right frames are not displayed at all. If the device is switched to landscape and then back to portrait, the font inflation issues seems to be fixed. NYTimes: the webpage looks fine both in portrait in landscape. If the page orientation is changed, no changes will occur regarding the font inflation. YouTube: the main frame which contain the videos list, looks really bad. Text is hyper inflated and rotating the device, will not fix the issue. It seems to be permanent, like for the Bugzilla case. Build 2 Bugzilla: font is inflated much more than the 1st build. If the device orientation is switched to landscape and back to portrait, will not resolve the font inflation issue. Google News: all 3 frames are displayed even from the 1st page load. The text from the main frame is a little bit more inflated than the side frames, but it looks OK. If the page is switched to landscape and then back to portrait, the text from the main frame seems to be smaller than it was just after the page was loaded, and also quite the same level as it on the left and right frame. So the Google News will look perfect in this case if the white space between the main frame and the right one will be reduced. NYTimes: the font seems to be a little bit less inflated than it is on the 1st build, but the difference is barely noticeable. The webpage looks fine YouTube: there is the same result as for the 1st build. Build 3 Bugzilla: the font seems to be even more inflated than it is for the first 2 builds. Again, there are no changes if the device orientation is switched to landscape and back to portrait. Google News: there are no words to describe how bad this page looks when it's loaded for the 1st time (see attached screenshots for build 3). After the device rotation, the font inflation issue still persist and the page still look really bad, NYTimes: the page's footer is highly inflated and also the Inside NYTimes.com section look horrible (we have to mention that the page cannot be panned on that side and things might be complicated when that section covers the entire screen) YouTube: there is the same result as for the 1st build. Conclusions: - Build 3 is far the worst test build. - Build 2 is better then the 1st one, because of the good layout shapes for Google News and NYTimes, but none of them could even candidate for the final release in our opinion - Bugzilla and YouTube pages looks awful on all 3 test builds, but as David said in bug 707195 c#36, a real pattern should be fixed, instead of fixing individual websites
So the summary of that is that so far we have 4 sites where builds (1) and (3) are known to differ: build build build (1) (2) (3) ycombinator comments BAD good good nytimes.com good good BAD Google News ? ? BAD Bugzilla ? ? ? (? marks are for where I can't tell from comment 40 which builds produce better results) So so far that provides no evidence that the compromise in build (2) applies any more broadly than the two sites for which the condition was designed (ycombinator comments and nytimes.com). But per discussion in the triage meeting today, more testing is in progress.
The following websites were tested and might be helpful to find a solution for this issue: http://en.wikipedia.org/w/index.php?title=Dark_side http://support.microsoft.com/ph/14019 http://www.apple.com/itunes/ http://forum.xda-developers.com/showthread.php?t=794444 http://wordpress.org/ http://www.android.com/about/ice-cream-sandwich/ http://www.imdb.com/
Some additional gatherings — albeit a bit inconclusive, I have ran through a variety of varied top-sites by country which serve desktop-optimized content across all posted builds. The content served has markup which is not greatly affected by the font-inflation heuristics tweaked in each build with the exception of some greatly noticeable differences in Bugzilla and Y Combinator and Drudge Report. http://people.mozilla.com/~atrain/mobile/Evangelism/font-inflation-compare/
(In reply to Aaron Train [:aaronmt] from comment #43) > Some additional gatherings — albeit a bit inconclusive, I have ran through a > variety of varied top-sites by country which serve desktop-optimized content > across all posted builds. The content served has markup which is not greatly > affected by the font-inflation heuristics tweaked in each build with the > exception of some greatly noticeable differences in Bugzilla and Y > Combinator and Drudge Report. > > http://people.mozilla.com/~atrain/mobile/Evangelism/font-inflation-compare/ And I think only Bugzilla is negatively affected. Y-Combinator and Drudge Report are more readable with the inflation.
removing qawanted tag, as comment 40 provided testing. re-nom the qawanted if there's more test builds you'd like investigation on.
P2 per font inflation scrub.
Assignee: dbaron → sjohnson
Priority: P1 → P2
Apologies if I'm stating the obvious, since I can't see this explicitly mentioned: don't the tiny comments on ycombinator tend to be the shorter ones? i.e. the different sizes have more to do with the length of the comment than the nesting depth. This pattern holds for e.g. phoronix.com forums and guardian comments (bug 752429), neither of which have nested threads (and the guardian comments page uses floats not tables). Take http://news.ycombinator.com/item?id=4291084 as a recent example. I see shrunken comments at all nesting levels. Additionally, take the direct link to this comment: http://news.ycombinator.com/item?id=4293161 which at the time of posting has no replies so there's only one level of nesting, and it's still shrunken and hard to read without zooming in. So based on my inexpert reading of http://dev.w3.org/csswg/css-size-adjust/ isn't the problem with comments too short to trigger text inflation?
(In reply to Oliver Henshaw from comment #50) > Apologies if I'm stating the obvious, since I can't see this explicitly > mentioned: don't the tiny comments on ycombinator tend to be the shorter > ones? i.e. the different sizes have more to do with the length of the > comment than the nesting depth. This pattern holds for e.g. phoronix.com > forums and guardian comments (bug 752429), neither of which have nested > threads (and the guardian comments page uses floats not tables). > > Take http://news.ycombinator.com/item?id=4291084 as a recent example. I see > shrunken comments at all nesting levels. Additionally, take the direct link > to this comment: http://news.ycombinator.com/item?id=4293161 which at the > time of posting has no replies so there's only one level of nesting, and > it's still shrunken and hard to read without zooming in. > > So based on my inexpert reading of http://dev.w3.org/csswg/css-size-adjust/ > isn't the problem with comments too short to trigger text inflation? Oliver: Yes, you're correct. It has to do with table cells being assigned as font-inflation "flow roots", which is what we call a node in the frame tree where font inflation data aggregation begins. So, in other words, what's happening is that at each table cell, we're making a decision about how much text is underneath the given flow root and determining whether we should inflate based on this. Unfortunately, since every table cell is a flow root, it means that we're restarting aggregation of font inflation data for each table cell, when we should be doing it for the whole set of comments in total. The real crux of the issue, though, is that this is ONLY the case for situations where tables are being used for layout with respect to positioning - if a table is being used for representing tabular data, or to create a grid-like layout (as in the nytimes.com case), then we still want table cells to be marked as flow roots. We can't distinguish between these cases readily enough, because our system isn't designed to treat these two cases differently. We need to come up with an heuristic that allows us to distinguish these two cases.
jwir3 asked me to provide this screenshot of http://hg.mozilla.org/mozilla-central/pushloghtml This site also suffers from the separate inflation roots for individual table cells, although this page uses tables for tabular data, not presentation.
(In reply to Thomas Stache from comment #52) > Created attachment 649331 [details] > Screenshot hg.mozilla.org pushloghtml > > jwir3 asked me to provide this screenshot of > http://hg.mozilla.org/mozilla-central/pushloghtml > This site also suffers from the separate inflation roots for individual > table cells, although this page uses tables for tabular data, not > presentation. Thanks, Thomas!
This also seems to apply to anything based on Simple Machines Forum software. See http://www.simplemachines.org/community/index.php?topic=483398.0 as an example This page displays fine on a Firfox desktop but some items have tiny text using Android ARM on my HTC One V (ICS) or Nexus 7 (Jelly Bean).
(In reply to dggrr from comment #54) > This also seems to apply to anything based on Simple Machines Forum software. Default phpBB3 theme as well (prosilver), and I think I've also seen it on Slashdot.
Some of my most frequented online forums suffer from this bug; they are indeed phpBB based so I guess that posting more URLs doesn't make sense but this defect is for me probably the biggest reason why I'm not using more FireFox on Android. The Android native browser and Dolphin (which uses the native browser engine) do not suffer from this same problem (at least not visibly to my eye!). I have also never observed this issue on desktop.
I'm new to Firefox for Android, as I own an ARMv6 mobile, and in my opinion this is the biggest problem. It makes a lot of pages barely readable. (That's too bad because, for the rest, I love having Sync in my pocket)
I submitted this bug before and it was marked as a dup in this thread. I just wanted to give a big thanks to everyone working hard to fix this. Everything you're doing is appreciated. David, Patrick, Sam, Scott, Emanuel, Brad, JP, Ian, Jet, Christian, Aaron, Tony, Oliver, and anyone I might have missed. Thank you
For me every page mentioned works and is displayed as it should. To achieve that I have set the font size to tiny and enabled fit text size after zoom. I have additionally set browser.viewport.defaultZoom pref to 1000 in about:config Try it out!
I had a similar problem with Firefox 25.0 on a Nexus 7. Setting "-moz-text-size-adjust: none;" in the CSS does the trick for me!
Whiteboard: [font-inflation: nested tables][readability] → [font-inflation: nested tables][readability][mentor=dbaron][badfirstbug]
Whiteboard: [font-inflation: nested tables][readability][mentor=dbaron][badfirstbug] → [font-inflation: nested tables][readability][badfirstbug]
Thank you Alexander for that CSS rule. I added it to my pages and Firefox handles them in a saner way. However any other browser handles font inflation in a better way by default. This is my biggest complaint about FF on Android. To make developers understand how important it is, this is the reason why I don't use FF on Android even if it is my primary desktop browser and this is the reason why I won't neither consider nor recomment a Firefox OS phone: basically I'm sure that a Firefox OS phone can't be used to browse the web. Nevertheless, thanks for trying but please find a solution.
One of a most annoying "features". Example: http://www.overclockers.ru/hardnews/63102/Vypuskat_planshety_stanovitsya_vse_menee_vygodno.html
(In reply to Eugene Savitsky from comment #64) > One of a most annoying "features". Example: > http://www.overclockers.ru/hardnews/63102/ > Vypuskat_planshety_stanovitsya_vse_menee_vygodno.html In the comments area at the bottom.
No assignee, updating the status.
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.