Closed Bug 713241 Opened 8 years ago Closed 8 years ago

Font inflation on inflates the footer too much


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




Tracking Status
blocking-fennec1.0 --- +
fennec 11+ ---


(Reporter: blassey, Assigned: jwir3)




(Whiteboard: [readability])


(2 files)

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

If I open 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
OS: All → Android
Hardware: All → ARM
tracking-fennec: --- → 11+
Whiteboard: [readability]

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

1) The screenshot is an example of how'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. 

Assignee: nobody → sjohnson
Depends on: 706193
Duplicate of this bug: 718773
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 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.
That won't work -- see bug 706193 comment 5, particularly the third paragraph.
(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? ;)

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

Attachment #593568 - Flags: review?(matspal)
Attachment #593568 - Flags: review?(matspal) → feedback?(matspal)
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...
With 02/01 Nightly (13.0a1) on, I still see a abnormally sized copyright footer.
(In reply to Aaron Train [:aaronmt] from comment #8)
> With 02/01 Nightly (13.0a1) on, 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 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.
Attachment #593568 - Flags: feedback?(matspal)
Great. Thanks for the feedback, mats. I'll try working something like this out!
Blocks: 714080
Blocks: 718501
Release blockers.
blocking-fennec1.0: --- → +
No longer blocks: 714080
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.
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 706193
You need to log in before you can comment on or make changes to this bug.