713241 - Font inflation on nightly.mozilla.org inflates the footer too much

Status: RESOLVED DUPLICATE of bug 706193

(Firefox for Android Graveyard :: General, defect, P1)

Description

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

Attachment: screenshot of nightly.mozilla.org's footer

+++ This bug was initially created as a clone of Bug #706151 +++

If I open nightly.mozilla.org the only text inflated is the Privacy Policy/License info in the footer, which is intentionally tiny on the desktop. The main page content stays zoomed out in small text.


HTC Thunderbolt :: Gecko/20111128 Fennec 11.0a1::Android 2.3.4

Comment 1

[Scott Johnson (:jwir3)]

Brad,

I'm a bit confused here. I interpret this bug as meaning one of the following:

1) The screenshot is an example of how nightly.mozilla.org's footer looks with default font inflation settings, and we need to tweak the defaults for this to look better.

2) The font inflation is only working on the text in the footer, and the code for font inflation needs to be changed so that the rest of the text on the page is inflated for more readability.

3) The text in the footer should not be inflated, and the code should be changed so that the text in the footer looks the same as the text in the rest of the page. 

Can you clarify which of these three you expect to assist in resolving the issue? I'm just trying to get a feel for which direction to proceed. 

Thanks!

Comment 3

[Scott Johnson (:jwir3)]

I decided to go about this in a way suggested by dbaron - namely, I added a character threshold preference for font-inflation. If the number of characters in a container (including it's sub-containers) is less than this amount, then font inflation will be disabled for that container. I set the default to 200 for the time being, but this could change. The reason I chose 200 was that it seemed to work well with the nightly.mozilla.org footer text, but not suppress a larger amount of text, in say, news articles. 

This won't solve the larger problem, as I think we need to see if we can find a way to do two things (note this is just a rough strategy, not something I'm sure will work):

1) Develop a heuristic for determining if something is a horizontal "list" (e.g. like a nav menu). This would probably be of the pattern <text> <separator> <other text> <same separator>, etc... where the separator would likely be a punctuation character, or set of punctuation characters (such as "|" or ">>"). This is probably a hard problem.

2) See if we can determine, with some probability, whether text is a "copyright" notice. This would definitely be a hack, but it might help us to eliminate the vast number of "footers" that are getting abnormally inflated.

Comment 4

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

That won't work -- see bug 706193 comment 5, particularly the third paragraph.

Comment 5

[Scott Johnson (:jwir3)]

(In reply to David Baron [:dbaron] (busy through 12 Feb) from comment #4)
> That won't work -- see bug 706193 comment 5, particularly the third
> paragraph.

By 'that' do you mean the character threshold or the two crazy idea(s) I had about fixing this by probabilistically determining semantic context? ;)

Comment 6

[Scott Johnson (:jwir3)]

Attachment: b713241 - Add a minimum character threshold for font inflation to work.

Mats, 

I'm hoping you can take a look at this and see if I am doing things correctly.

Thanks!

Comment 7

[Scott Johnson (:jwir3)]

So, one thing I didn't anticipate is that if we have a news article with some X number of characters in it, and with a header having Y characters of text, where X > 200 > Y, it might look strange because the header won't be inflated, but the article will be inflated. 

I'm still thinking about how this might work...

Comment 8

[Aaron Train [:aaronmt]]

With 02/01 Nightly (13.0a1) on apple.com, I still see a abnormally sized copyright footer.

Comment 9

[Scott Johnson (:jwir3)]

(In reply to Aaron Train [:aaronmt] from comment #8)
> With 02/01 Nightly (13.0a1) on apple.com, I still see a abnormally sized
> copyright footer.

Hm, yes I think this is expected at this point. This patch isn't anywhere close to landing yet... I'm just sort of thinking through a possible avenue for the solution.

Comment 10

[Mats Palmgren (inactive)]

Comment on attachment 593568 [details] [diff] [review]
b713241 - Add a minimum character threshold for font inflation to work.

I'm worried that the patch as written would be too slow, I don't think
we can afford frequent calls to GetTextContent() during reflow.
Maybe doing it once might be OK and then caching the result? On trunk
it might work to null out presContext->mCurrentInflationContainer,
and/or use a frame bit?
Traversing frame descendants is probably cheaper than GetTextContent()
since you can stop after finding the minimum amount.  Also, you'd get
some control over which frames to traverse, in case you want to skip
out-of-flow frames for example.

Comment 11

[Scott Johnson (:jwir3)]

Great. Thanks for the feedback, mats. I'll try working something like this out!

Comment 12

[JP Rosevear [:jpr]]

Release blockers.

Comment 13

[Scott Johnson (:jwir3)]

I'm fairly confident that this is a duplicate of bug 706193. Please reopen if I'm incorrect. Note that we're going to want to test this when bug 7061934 lands.