Bug 627842 (font-inflation)

Allow minimum font size based on size of frame

RESOLVED FIXED in mozilla11

Status

()

Core
Layout: Text
P3
normal
RESOLVED FIXED
7 years ago
4 months ago

People

(Reporter: stechz, Assigned: dbaron)

Tracking

(Depends on: 18 bugs, {dev-doc-complete})

Firefox Tracking Flags

(blocking2.0 -, fennec+)

Details

Attachments

(19 attachments, 13 obsolete attachments)

13.37 KB, patch
Details | Diff | Splinter Review
2.88 KB, patch
roc
: review+
Details | Diff | Splinter Review
6.17 KB, patch
roc
: review+
Details | Diff | Splinter Review
4.11 KB, patch
Details | Diff | Splinter Review
3.86 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.26 KB, patch
roc
: review+
Details | Diff | Splinter Review
6.62 KB, patch
roc
: review+
Details | Diff | Splinter Review
6.78 KB, patch
roc
: review+
Details | Diff | Splinter Review
20.16 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.32 KB, patch
roc
: review+
Details | Diff | Splinter Review
5.33 KB, patch
roc
: review+
Details | Diff | Splinter Review
2.13 KB, patch
mats
: review+
Details | Diff | Splinter Review
19.73 KB, patch
roc
: review+
Details | Diff | Splinter Review
2.22 KB, patch
roc
: review+
Details | Diff | Splinter Review
20.02 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.46 KB, patch
roc
: review+
Details | Diff | Splinter Review
3.57 KB, patch
roc
: review+
Details | Diff | Splinter Review
80.84 KB, patch
roc
: review+
Details | Diff | Splinter Review
25.13 KB, patch
roc
: review+
Details | Diff | Splinter Review
(Reporter)

Description

7 years ago
This is for readable text in Fennec. Enforcing a minimum font size often breaks page layouts, but what if we only enforce minimum sizes for text that has line breaks and base the font size on the width of the block that contains the text? This would be much less aggressive, and would allow minimum font sizes that, while too small to be read zoomed out, work just fine once you are zoomed in to the text (without having to pan back and forth).
"Text with line breaks" --- you mean white-space:normal and pre-wrap?

Would you need to also check that the containing block can expand in height somehow?
(Reporter)

Comment 2

7 years ago
(In reply to comment #1)
> "Text with line breaks" --- you mean white-space:normal and pre-wrap?

Sort of, but I mean any text that wraps. For instance, a white-space: normal <div> with a small bit of text that doesn't wrap would not be affected.

> Would you need to also check that the containing block can expand in height
> somehow?

That would be a good check, though I don't know how necessary it would be in practice.

--

I tried changing the font's size in GetFontMetricsForFrame, but this seems to happen after all of the text layout had been determined (line height is computed much earlier, and any links or spans within the text run would scrunch together).

The only approach I've come up with is rather hacky: change the actual style of the content node to use a different font size at a spot where we know the size of the text frame, and let the reflow code do its thing. We'd have to watch out for any blocks defined in "em" units.

Ideally, I hope there's some place between knowing the size of the text frame and actually performing the text run where I could change the font size and then let the frame recalculate its height and do the text run.

Roc: do you have any idea where to start on something like this?
This is our only proposal so far to fix problems with our current textZoom-based reflow implementation (see bug 611555 and dependencies).  Nominating for blocking-fennec.
Assignee: nobody → ben
Blocks: 623831, 623820, 627103
Status: NEW → ASSIGNED
tracking-fennec: --- → ?
(In reply to comment #2)
> Sort of, but I mean any text that wraps. For instance, a white-space: normal
> <div> with a small bit of text that doesn't wrap would not be affected.

Hmm, that adds complexity...

> I tried changing the font's size in GetFontMetricsForFrame, but this seems to
> happen after all of the text layout had been determined (line height is
> computed much earlier, and any links or spans within the text run would scrunch
> together).

GetFontMetricsForFrame is used for textrun construction, so that should work in terms of getting the right horizontal layout for spans. I would want to understand why it does not.

For vertical dimensions, the line-height should not be fixed until we've reflowed all the text, unless you're using the line-height property with values other than 'normal'. So you would want to check 'line-height' on the containing block and not do your magic resizing if line-height is other than 'normal' (the default).

> Roc: do you have any idea where to start on something like this?

GetFontMetricsForFrame was the right place to start.

You'll need to get dynamic changes right. When your text-zoom is enabled, nsBlockFrame::PrepareResizeReflow will need tryAndSkipLines to be false always.

Regarding your first comment, you can check mLineBreaker in BuildTextRunForFrames and pass something down to GetFontMetricsForFrame to let it know if there's a break opportunity in this text. You might get weird results though, e.g.
<b>He</b>llo Kitty
The first text frame won't know about any break opportunities, so it will be normal-sized, bu the second text frame will know about break opportunities so it can be resized. That would look terrible.

Perhaps you need to decide in nsBlockFrame::Reflow whether you're going to enable text resizing for the entire block or not. You could instantiate your own nsLineBreaker and gather all the text content and look for break opportunities, although that's going to mean duplicating some code for text layout :-(.

Hmm, another option would be to do two-pass reflow. How about this: optimistically assume that text resizing is OK, reflow the block with text resizing enabled, and then detect if there's overflow. If there is, disable text resizing and reflow again. No messing around with line breaking etc. How does that sound? Making this work with intrinsic width calculations (for table cells and floats) sounds hard ... can you get away with disabling text resizing in those situations?
BTW is this really a blocking bug? Because this is a major and potentially destabilizing new feature. Is this really a good place in the Fennec schedule to be doing this?
(Reporter)

Comment 6

7 years ago
After talking it out and thinking it over last night, I'm not the right person for this patch. Putting this bug back to unassigned.
Assignee: ben → nobody
Status: ASSIGNED → NEW

Comment 7

7 years ago
what roc said.
tracking-fennec: ? → 2.0next+

Updated

7 years ago
tracking-fennec: 2.0next+ → 2.0+

Comment 8

7 years ago
Would have been great to get this in sooner, but is something that we need to really get text to be readable.
Assignee: nobody → pavlov

Updated

7 years ago
Assignee: pavlov → nobody
blocking2.0: --- → ?

Comment 9

7 years ago
We need to get this in as soon as possible so that we can get people testing it.
blocking2.0: ? → -
The last paragraph of comment #4, in more detail:
1) Add a state bit on nsBlockFrame to indicate whether "text resizing" is enabled for that block.
2) nsTextFrame::Reflow checks that bit on the nearest ancestor block. When it's set, allow GetFontMetricsForFrame to increase the text size to some minimum size. Record in the textframe state bits whether we have increased the text size.
3) [optional] when that bit is set, treat CSS line-height as "normal" no matter what the style says
4) When nsBlockFrame::Reflow finishes reflowing its lines, check whether the block contents overflow the block's content-box. (We may also need to check the content-boxes of ancestor frames here ... this needs more thought.) If there is overflow, check the children to see if any had their text size increased by text resizing. If they did, disable text resizing, dirty all lines (possibly just the lines containing text whose size was increased) and reflow the block contents again.
Created attachment 514264 [details] [diff] [review]
WIP m-c patch
Attachment #514264 - Flags: feedback?(roc)
Created attachment 514266 [details] [diff] [review]
WIP m-b patch
Assignee: nobody → blassey.bugs
(In reply to comment #11)
> Created attachment 514264 [details] [diff] [review]
> WIP m-c patch

Looks OK but doesn't do any of the hard layout stuff obviously :-)
right, this is cheating and doing the hard stuff on double tap in the front end
Roc,changing the font size in GetFontMetricsForFrame doesn't seem to have any effect on the eventual layout of the page. I've mucked around a bit experimenting with other approaches, mostly using the previous patch and setting the min font size on the nsPresContext at various points in the layout/reflow of a block frame. That does have an effect on the layout, but winds up not doing what we want and most the time results in an eventual crash (typically when dealing with border images).
Created attachment 515507 [details] [diff] [review]
hack patch

This patch hacks GetFontMetricsForFrame to use a font size of 100px. It definitely affects the layout of the UI, and if I load

<p>Hello <b>Kitty</b>
<p>Hello

it gets laid out "properly".
Created attachment 515558 [details] [diff] [review]
WIP m-c patch

roc, thanks for the pointer. 

This patch sets a min font size based on the width of the block frame or first ancestor block frame. The value of that minimum is based on a scale factor saved in a pref.

With this, the font sizes are changing, but the layouts get out of whack (especially for larger font sizes). It looks like we may not be measuring line height correctly.
Attachment #514264 - Attachment is obsolete: true
Attachment #515558 - Flags: feedback?(roc)
Attachment #514264 - Flags: feedback?(roc)
Created attachment 515559 [details] [diff] [review]
WIP m-b patch

this sets the pref mentioned for the m-c patch based on the window width. The scale factor was chosen unscientifically by trial and error.
Attachment #514266 - Attachment is obsolete: true
+        nsStylePosition* pos = new (pc) nsStylePosition();
+        //const nsStylePosition *stylePos = bFrame->GetStylePosition();
+        nscoord width;
+        GetIntrinsicCoord(pos->mWidth, rc, bFrame, PROP_WIDTH, width);

Why create a new nsStylePosition here?

On what testcases are you not getting good line heights? I mentioned above that any pages that set CSS line-height to something other than 'normal' are not compatible with this approach.
I'm using the nsStylePositon to get an nsStyleCoord to pass into GetIntrinsicCoord(). Calling bFrame->GetPrefWidth(rc) crashes for some unknown reason
Created attachment 515613 [details] [diff] [review]
WIP m-c patch

still just mucking with this unscientifically, just checking if the parent is a block rather than searching for first block ancestor seems to work better
Attachment #515558 - Attachment is obsolete: true
Attachment #515558 - Flags: feedback?(roc)
> I'm using the nsStylePositon to get an nsStyleCoord

Uh... but that's just passing in the initial value (so "auto") given your code.  What's the point of that?  If you really just want to pass auto, pass auto explicitly instead of creating (and leaking, too) an nsStyleCoord.

> Calling bFrame->GetPrefWidth(rc) crashes for some unknown reason

Unknown in what sense?  No stack?
(In reply to comment #22)
> > I'm using the nsStylePositon to get an nsStyleCoord
> 
> Uh... but that's just passing in the initial value (so "auto") given your code.
>  What's the point of that?  If you really just want to pass auto, pass auto
> explicitly instead of creating (and leaking, too) an nsStyleCoord.

that sounds better, thanks

> > Calling bFrame->GetPrefWidth(rc) crashes for some unknown reason
> 
> Unknown in what sense?  No stack?

there's a stack and its a seg fault, but its not clear to me why. The top of the stack is bFrame->GetPRefWidth(rc) but bFrame and rc are both valid
Created attachment 515638 [details] [diff] [review]
WIP m-c patch
Attachment #515613 - Attachment is obsolete: true
Can you attach the stack here?
Assignee: blassey.bugs → nobody
No longer blocks: 623831
(Assignee)

Comment 26

6 years ago
Comment on attachment 515638 [details] [diff] [review]
WIP m-c patch

A few notes based on skimming through this patch:

 * nsHTMLReflowState doesn't need a new mFrame; it already has frame

 * CalcLineHeight and ComputeLineHeight should have the style context parameter replaced with a frame parameter, rather than the frame parameter added.  However, you'd need something else for nsComputedDOMStyle, into which this introduces a null-dereference crash, and which shouldn't honor this pref anyway.

 * the tree changes break tree styling pseudo-classes; GetFontMetricsForStyleContext should stay for the SVG and the tree code


That said, I think it's crazy to be trying this at this stage of a release:  this is something that's going to require user feedback both as to whether the algorithm is good and as to whether it doesn't break too much Web content.
(In reply to comment #26)
> That said, I think it's crazy to be trying this at this stage of a release: 
> this is something that's going to require user feedback both as to whether the
> algorithm is good and as to whether it doesn't break too much Web content.

I couldn't agree more
(Assignee)

Comment 28

6 years ago
Also, I don't see what prevents this from causing infinite-growing loops when handling dynamic changes in intrinsically-sized content.
(Assignee)

Comment 29

6 years ago
resetting this from + to ? to trigger reevaluation based on above comments
tracking-fennec: 2.0+ → ?

Comment 30

6 years ago
We really have to have this -- text is simply too small to read in most cases and we can't ship without it
tracking-fennec: ? → 2.0+
Whiteboard: [at-risk]
(Assignee)

Comment 31

6 years ago
(In reply to comment #0)
> This is for readable text in Fennec. Enforcing a minimum font size often breaks
> page layouts, but what if we only enforce minimum sizes for text that has line
> breaks and base the font size on the width of the block that contains the text?
> This would be much less aggressive, and would allow minimum font sizes that,
> while too small to be read zoomed out, work just fine once you are zoomed in to
> the text (without having to pan back and forth).

For what it's worth, this is still substantially more aggressive than what mobile Safari does.  It seems like mobile Safari does scaling up of text, dependent on the width of the block, when there are multiple (not necessarily consecutive) blocks within the same normal flow that have the same font-size and font-family.  Or something like that -- there are probably a bunch more conditions.  It's not a minimum font size, since the specified font size still matters.

But that's also substantially more complicated to implement.


So maybe we could try this -- but there's certainly a risk that it's going to break pages given that it's more aggressive than what others are doing.
(Assignee)

Comment 32

6 years ago
(In reply to comment #28)
> Also, I don't see what prevents this from causing infinite-growing loops when
> handling dynamic changes in intrinsically-sized content.

It looks like what mobile Safari does is that it does shrink-wrapping width calculations at the non-grown font size, and then grows the font size based on the resulting width.  That's involved enough and breaks enough of our invariants that there's no way it's going to happen for this release (never mind the fact that I'm mostly unavailable the next week and a half).
(Assignee)

Comment 33

6 years ago
(In reply to comment #32)
> (In reply to comment #28)
> > Also, I don't see what prevents this from causing infinite-growing loops when
> > handling dynamic changes in intrinsically-sized content.
> 
> It looks like what mobile Safari does is that it does shrink-wrapping width
> calculations at the non-grown font size, and then grows the font size based on
> the resulting width.  That's involved enough and breaks enough of our
> invariants that there's no way it's going to happen for this release (never
> mind the fact that I'm mostly unavailable the next week and a half).

I'm starting to think that doing the growing based on the container's width without this is probably hard, and it would be safer to base the growing on the amount of text (within a given BFC).  That complicates incremental layout, though.  (Either way is probably a substantial performance penalty.)

Comment 34

6 years ago
Could we do this as some sort of external pass post-reflow?  I realize it wouldn't be as efficient, but might get us what we want?

Comment 35

6 years ago
minusing for release -- would really like to get this in a short follow up (4.0.1?) if at all possible.  We're going to work on a compromise user pref to do some text reflow in the meantime
tracking-fennec: 2.0+ → 2.0next+
Blocks: 642519
Whiteboard: [at-risk] → [fennec-6]
dbaron, I was told you were looking at this. If that's not true, please let me know
Assignee: nobody → dbaron
tracking-fennec: 2.0next+ → 6+
Whiteboard: [fennec-6]
tracking-fennec: 6+ → 7+
tracking-fennec: 7+ → 8+
(Assignee)

Updated

6 years ago
Depends on: 678671

Updated

6 years ago
tracking-fennec: 8+ → 9+
(Assignee)

Comment 37

6 years ago
Created attachment 564668 [details] [diff] [review]
work-in-progress part 3: apply font size inflation to text

I wanted to get your thoughts on this part of the patch before going too far down this path, which turns out to be a little messy.

The basic idea of what I'm planning is that the font size inflation has to be applied directly by the frames since it needs knowledge of container widths, so it has to happen both after style data computation and after intrinsic size calculation (which the text frame does part of).

My current approach is to do the reflow side of this inflation manually, and the painting side via a scale in the graphics context, though I'm open to other suggestions.  The frame's rect obviously has to include the scale, and I'm currently thinking mAscent should match the rect (though it could go either way).  It may be a little hard, and a source of further bugs, to disentangle which parts of the code want uninflated sizes and which parts (such as everything exposed to the outside of nsTextFrame) want inflated sizes.

I'm wondering if you have any thoughts on this approach or ideas to improve it.

If you're interested in the state of the other pieces, my full patch queue for this bug is:
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/4edb647579db/text-size-adjust
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/4edb647579db/font-size-inflation-base
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/4edb647579db/font-size-inflation-text
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/4edb647579db/remove-unused-ctx-param
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/4edb647579db/font-size-inflation-line-height
Attachment #564668 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #564668 - Flags: review?(roc)
Attachment #564668 - Flags: review?
Attachment #564668 - Flags: feedback?(roc)
(Assignee)

Updated

6 years ago
Attachment #564668 - Flags: review? → feedback?(roc)
(Assignee)

Updated

6 years ago
Attachment #564668 - Flags: feedback?(roc)
+  // REVIEW: He're we're assuming that drawing text with a transform
+  // scales it up by exactly the scale in the transform.  Given hinting,
+  // is that a safe assumption?  (It drastically simplifies this code,
+  // since we don't need two different text runs to implement font size
+  // inflation.)

Yeah, that's not a safe assumption and doing this will probably result in ugly text.

I think instead of trying to use the same textrun and scaling up during paint, we should compute an effective font size in Reflow() and do everything based on that effective font size, including generating a new text run if necessary. I think we should stop referring directly to mTextRun, and change EnsureTextRun to return a textrun as well as the iterator, and add a parameter to EnsureTextRun to indicate whether the requested textrun is the "normal" or "possibly-inflated" textrun. We should probably store the inflated textrun separately, if it's different from mTextRun, probably in a frame property (alongside the inflation factor). I would prefer to juggle two possibly-different textruns than to sometimes multiply or divide by the inflation factor.

Does that make sense?
(Assignee)

Updated

6 years ago
Attachment #564668 - Flags: feedback?(roc) → feedback-
(Assignee)

Comment 39

6 years ago
Sounds reasonable, though I'm still wrapping my head around some of the text run caching code (which I think will need some changes for text frames to be associated with two different text runs which might expire at different times).

One difference, though:  my inclination would be that when the inflated and normal text runs are different, the inflated text run would be the mTextRun, and the un-inflated text run would be the odd one out (since it's only needed in GetMinWidth and GetPrefWidth, which may well never be called).
Makes sense.
(Assignee)

Comment 41

6 years ago
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dbaron@mozilla.com-5da917ddb6b9/try-android/ has a test build based on the patches:

https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/723c04218e82/text-size-adjust
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/723c04218e82/constrained-height-bit
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/723c04218e82/font-size-inflation-base
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/723c04218e82/use-text-run-for-tab-width
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/723c04218e82/font-size-inflation-text
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/723c04218e82/remove-unused-ctx-param
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/723c04218e82/font-size-inflation-line-height

I haven't actually tried them out on a device yet, and I have no idea if the inflation factor is anywhere near reasonable (but it's a pref -- and the only one in about:config with "inflation" in the name -- the pref just gives the number of square characters per line that would be readable, with 0 disabling the inflation feature).  (I also want to try an alternative strategy for specifying the pref that might be less device-specific.)

There are a bunch of pretty obvious bugs (most obviously, with backgrounds/borders on inline elements and the size of text controls) that I want to work on today, then move on to implementing code to determine the inflation factor from the device size, and then look into further tuning of the heuristic.
(Assignee)

Comment 42

6 years ago
updated try build at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dbaron@mozilla.com-4a1ca43eaef8/try-android/
(Assignee)

Comment 43

6 years ago
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dbaron@mozilla.com-376690d5e498/try-android/
(Assignee)

Comment 44

6 years ago
(In reply to David Baron [:dbaron] from comment #43)
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dbaron@mozilla.com-
> 376690d5e498/try-android/

So I think it may be interesting for a small number of people to test this build.  There are two different preferences for enabling font size inflation; this build has it enabled with one of them, but it's more likely the other that will be more interesting (and I think it may even work as intended now, though verifying that will require a little more testing across different devices).  The preferences are described here:
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/file/95399627c734/font-size-inflation-base#l346
interesting values for "font.size.inflation.emPerInch" are probably in the 8-10 range; interesting values for "font.size.inflation.emPerLine" are probably in the 20-30 range.  (That build has emPerLine set to a default of 30; setting emPerInch is probably a more interesting pref default, assuming it works.)

I'd note that I haven't figured out what the right thing to do with textfields is yet -- though I'm quite sure the current behavior isn't it.
I've tested the build from comment 44 a little bit on my LG Optimus Black.
http://people.mozilla.com/~mwargers/tests/627842_minfontsize.htm
I noticed that with this build, the font in the first block gets inflated, but not with the second, small width block. I think that is expected with this build.

I think I noticed a problem with this build on http://news.google.com/m/news?ned=us
See screenshot:
http://people.mozilla.com/~mwargers/tests/627842_minfontsize.png
Certain links (the green links and the section links at the bottom) get inflated with this build.
The "Search" placeholder text in the filter input is too big with this build on the LG Optimus Black: http://people.mozilla.com/~mwargers/tests/627842_minfontsize_aboutconfig.png
Ok, I guess that is a problem with text fields in general as dbaron already noted in comment 44:
http://people.mozilla.org/~mwargers/tests/627842_minfontsize_inputs.htm
(Assignee)

Comment 48

6 years ago
(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #45)
> I've tested the build from comment 44 a little bit on my LG Optimus Black.
> http://people.mozilla.com/~mwargers/tests/627842_minfontsize.htm
> I noticed that with this build, the font in the first block gets inflated,
> but not with the second, small width block. I think that is expected with
> this build.
> 
> I think I noticed a problem with this build on
> http://news.google.com/m/news?ned=us
> See screenshot:
> http://people.mozilla.com/~mwargers/tests/627842_minfontsize.png
> Certain links (the green links and the section links at the bottom) get
> inflated with this build.

So this is an issue that I knew about but didn't realize would be as big of a problem:  that the code doesn't currently respond correctly to page resizes.  Since text size now depends on the block width, resizing a block needs to force reflow of all of its descendants.  I have a FIXME in my queue to do this.

The reason I think it's a problem is that we initially load the page at the default viewport width, then detect something (not sure what, though maybe it serves a <meta ... viewport> to Fennec but not desktop Firefox) that makes us resize the viewport narrower.

(In reply to Martijn Wargers [:mw22] (QA - IRC nick: mw22) from comment #46)
> The "Search" placeholder text in the filter input is too big with this build
> on the LG Optimus Black:
> http://people.mozilla.com/~mwargers/tests/627842_minfontsize_aboutconfig.png

This is the issue with text inputs that I mentioned.
(Assignee)

Comment 49

6 years ago
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dbaron@mozilla.com-bc948761af9f/try-android/ will soon have another round of builds with those issues fixed.  This will be a build based on:
https://hg.mozilla.org/mozilla-central/rev/3c684b4b8f68
plus the patches:
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/text-size-adjust
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/constrained-height-bit
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/font-size-inflation-base
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/resize-cause-dirty
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/get-font-metrics-inflation
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/use-text-run-for-tab-width
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/font-size-inflation-text
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/remove-unused-ctx-param
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/font-size-inflation-line-height
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/font-size-inflation-reftests
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/font-size-inflation-inline
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/font-size-inflation-bullets
https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/b599ff3b937b/font-size-inflation-other
tracking-fennec: 9+ → +
With the build from comment 49, the font size of about:home seems to be inflated, see:
http://people.mozilla.org/~mwargers/tests/627842_minfontsize/abouthome_minfontsizebuild.png
and compare with Aurora on the same LG Optimus Black phone:
http://people.mozilla.org/~mwargers/tests/627842_minfontsize/abouthome_aurora.png

Also, on http://news.google.com/m/news?ned=us , the fonts look bigger with this special build:
http://people.mozilla.org/~mwargers/tests/627842_minfontsize/googlenewsm_minfontsizebuild.png
Compare to the latest Aurora build:
http://people.mozilla.org/~mwargers/tests/627842_minfontsize/googlenewsm_aurora.png
Also compare this build with a regular build on the file:/// page.
http://people.mozilla.org/~mwargers/tests/627842_minfontsize/file_minfontsizebuild.png
http://people.mozilla.org/~mwargers/tests/627842_minfontsize/file_minfontsizebuild.png

Also seeing weird things on http://people.mozilla.org/~mwargers/tests/627842_minfontsize_inputs.htm
The text inputs seem to get bigger now, but the text is small in there, like in normal builds. Suddenly, when I type in the text inputs, the text gets huge.
(Assignee)

Comment 52

6 years ago
Created attachment 569858 [details] [diff] [review]
Add support for -moz-text-size-adjust CSS property.  (patch 1)
(Assignee)

Updated

6 years ago
Attachment #569858 - Attachment description: Add support for -moz-text-size-adjust CSS property. (, patch 1) property is analogous to the -webkit-text-size-adjust property (and → Add support for -moz-text-size-adjust CSS property. (patch 1)
Attachment #569858 - Flags: review?(bzbarsky)
(Assignee)

Comment 53

6 years ago
Created attachment 569860 [details] [diff] [review]
Set an NS_FRAME_IN_CONSTRAINED_HEIGHT state bit on frames that are in a constrained space.  (, patch 2)
(Assignee)

Updated

6 years ago
Attachment #569860 - Flags: review?(roc)
(Assignee)

Comment 54

6 years ago
Created attachment 569861 [details] [diff] [review]
When reflowing a frame (such as text controls) that jumps from HTML layout into XUL layout and then jumps back to HTML on the child frame, link the parent reflow state chain correctly.  (, patch 3)
(Assignee)

Comment 55

6 years ago
Created attachment 569862 [details] [diff] [review]
Implement computation of font size inflation for improved readibility of text on mobile devices.  (, patch 4)
(Assignee)

Comment 56

6 years ago
Created attachment 569863 [details] [diff] [review]
When font size inflation is enabled, horizontal resizes of blocks must cause a full dirty reflow.  (, patch 5)
(Assignee)

Comment 57

6 years ago
Created attachment 569864 [details] [diff] [review]
Add inflation parameter to nsLayoutUtils::GetFontMetricsFor* methods.  (, patch 6)
(Assignee)

Comment 58

6 years ago
Created attachment 569865 [details] [diff] [review]
Use the text run's font group for the tab width instead of recomputing it from the frame.  (, patch 7)
(Assignee)

Comment 59

6 years ago
Created attachment 569866 [details] [diff] [review]
Pass block's reflow state to nsTextFrame::UnionAdditionalOverflow.  (, patch 8)
(Assignee)

Comment 60

6 years ago
Created attachment 569867 [details] [diff] [review]
Apply font size inflation to text.  (, patch 9)
(Assignee)

Updated

6 years ago
Attachment #564668 - Attachment is obsolete: true
(Assignee)

Comment 61

6 years ago
Created attachment 569868 [details] [diff] [review]
Remove the unused context parameter to MeasureCharClippedText.  (, patch 10)
(Assignee)

Comment 62

6 years ago
Created attachment 569869 [details] [diff] [review]
Apply font size inflation to line heights.  (, patch 11)
(Assignee)

Comment 63

6 years ago
Created attachment 569870 [details] [diff] [review]
Apply font size inflation to heights of inlines.  (, patch 12)
(Assignee)

Comment 64

6 years ago
Created attachment 569871 [details] [diff] [review]
Apply font size inflation to list bullets.  (, patch 13)
(Assignee)

Comment 65

6 years ago
Created attachment 569872 [details] [diff] [review]
Pass nsFontMetrics to the GetEllipsis function rather than computing them again.  (, patch 14)
(Assignee)

Comment 66

6 years ago
Created attachment 569873 [details] [diff] [review]
Make other users of font metrics (other than MathML and XUL) honor font size inflation.  (, patch 15)
(Assignee)

Comment 67

6 years ago
Created attachment 569874 [details] [diff] [review]
Add reftest harness for testing font inflation and add reftests for basic features.  (, patch 16)
(Assignee)

Updated

6 years ago
Attachment #569861 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #569862 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #569863 - Flags: review?(bzbarsky)
(Assignee)

Updated

6 years ago
Attachment #569864 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #569865 - Flags: review?(matspal)
(Assignee)

Updated

6 years ago
Attachment #569866 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #569867 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #569868 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #569869 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #569870 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #569871 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #569872 - Flags: review?(matspal)
(Assignee)

Updated

6 years ago
Attachment #569873 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #569874 - Flags: review?(roc)
Attachment #569860 - Flags: review?(roc) → review+
Attachment #569861 - Flags: review?(roc) → review+
Comment on attachment 569862 [details] [diff] [review]
Implement computation of font size inflation for improved readibility of text on mobile devices.  (, patch 4)

Review of attachment 569862 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsLayoutUtils.cpp
@@ +4413,5 @@
> +    byLine = aContainerWidth / sFontSizeInflationEmPerLine;
> +  }
> +  if (sFontSizeInflationMinTwips != 0) {
> +    // REVIEW: Is this giving us app units and sizes *not* counting
> +    // viewport scaling?

Yes. You probably should include nsIPresShell::GetResolution here.

There's also a question of what, if anything, we should do about CSS transforms. I think probably content inside a CSS transform should not get font-size inflation.

@@ +4431,5 @@
> +// size of the same text; the aStyleFontSize parameter here should
> +// always be a font size, and never a line height.
> +/* static */ float
> +nsLayoutUtils::FontSizeInflationInner(const nsIFrame *aFrame,
> +                                      nscoord aMinFontSize)

No aStyleFontSize parameter here...

@@ +4491,5 @@
> +   * count as a container, so that, for example, an indented quotation
> +   * didn't end up with a smaller font size.  However, it's hard to
> +   * distinguish these situations where we really do want the indented
> +   * thing to count as a container, so we don't try, and blocks are
> +   * always containers.

It seems to me that the ideal thing here would be to have anything that's not in a line-of-inlines be a container for font-size inflation. What do you think?

::: modules/libpref/src/init/all.js
@@ +1555,5 @@
> + * A value greater than zero enables font size inflation for
> + * pan-and-zoom UIs, so that if a block's width is scaled to match the
> + * device's width, the fonts in a block are at least the font size
> + * given.  The value given is in twips, i.e., 1/20 of a point, or 1/1440
> + * of an inch.

I'm not sure twips is the right measure here. Why did you choose it? Seems to me we should use something that corresponds closer to viewing angle ... CSS pixels (modulo scaling)?
Attachment #569864 - Flags: review?(roc) → review+
Attachment #569866 - Flags: review?(roc) → review+
Comment on attachment 569867 [details] [diff] [review]
Apply font size inflation to text.  (, patch 9)

Review of attachment 569867 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsLineLayout.h
@@ +370,5 @@
>     */
>    nsIFrame* GetLineContainerFrame() const { return mBlockReflowState->frame; }
> +  const nsHTMLReflowState* GetLineContainerRS() const {
> +    return mBlockReflowState;
> +  }

This hunk should go in the previous patch.

::: layout/generic/nsTextFrame.h
@@ +388,5 @@
>                                       PRUint32* aFlowEndInTextRun = nsnull);
> +  // Since we can't reference |this| in default arguments:
> +  gfxSkipCharsIterator EnsureTextRun(bool aInflated) {
> +    return EnsureTextRun(aInflated,
> +                         aInflated ? mFontSizeInflation : 1.0f);

Boolean parameters suck ... Here probably we could have EnsureTextRun, EnsureTextRunNoInflation? And use an enum for the cases where we want to pass the text run type around as a parameter?

@@ +417,5 @@
> +  void ClearTextRun(nsTextFrame* aStartContinuation, bool aInflated);
> +
> +  void ClearTextRuns() {
> +    ClearTextRun(nsnull, true);
> +    ClearTextRun(nsnull, false);

Pass a mask to ClearTextRun so we don't have to do two passes?

@@ +455,5 @@
>    // so if we create a new continuation, this is where that continuation should
>    // start.
>    PRInt32     mContentLengthHint;
>    nscoord     mAscent;
> +  float       mFontSizeInflation;

I think this should go in a frame property, with a state bit to indicate its presence.
Attachment #569868 - Flags: review?(roc) → review+
Attachment #569869 - Flags: review?(roc) → review+
Attachment #569870 - Flags: review?(roc) → review+
Comment on attachment 569871 [details] [diff] [review]
Apply font size inflation to list bullets.  (, patch 13)

Review of attachment 569871 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsBulletFrame.h
@@ +119,5 @@
>  
>    nsSize mIntrinsicSize;
>    nsSize mComputedSize;
>    PRInt32 mOrdinal;
> +  float mFontSizeInflation;

Maybe this should be a property as well ... the same property as for text frames?
Attachment #569871 - Flags: review?(roc) → review+
Comment on attachment 569873 [details] [diff] [review]
Make other users of font metrics (other than MathML and XUL) honor font size inflation.  (, patch 15)

Review of attachment 569873 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsImageFrame.cpp
@@ +974,5 @@
>    // Set font and color
>    aRenderingContext.SetColor(GetStyleColor()->mColor);
>    nsRefPtr<nsFontMetrics> fm;
> +  nsLayoutUtils::GetFontMetricsForFrame(this, getter_AddRefs(fm),
> +    nsLayoutUtils::FontSizeInflationFor(this));

Maybe FontSizeInflationFor(nsIFrame*) should assert that the frame isn't being reflowed?
Attachment #569873 - Flags: review?(roc) → review+
Attachment #569874 - Flags: review?(roc) → review+
So if we use uninflated text sizes to determine intrinsic widths, then inflate, doesn't that mean a float or table cell could easily end up too small for its text content? Seems like maybe a container's inflation factor should have an upper bound of the container width divided by its intrinsic-min-width?
Comment on attachment 569858 [details] [diff] [review]
Add support for -moz-text-size-adjust CSS property.  (patch 1)

r=me
Attachment #569858 - Flags: review?(bzbarsky) → review+
Comment on attachment 569863 [details] [diff] [review]
When font size inflation is enabled, horizontal resizes of blocks must cause a full dirty reflow.  (, patch 5)

>other than InitResizeFlags and nsFrame::Box

nsFrame::BoxReflow.

>down to the cell, and once we hit the cell we'll set the code we've

"we'll hit the code"?  Or some other verb?  I have no idea what "set" means there.

The code changes seem ok, but can the text-size-adjust style lead to sitations where font size inflation is enabled on the prescontext but the width change doesn't trigger it?  If so, maybe a followup bug on detecting those cases and skipping the dirty reflow in them?

r=me with the above
Attachment #569863 - Flags: review?(bzbarsky) → review+

Updated

6 years ago
Attachment #569872 - Flags: review?(matspal) → review+
(Assignee)

Comment 75

6 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #72)
> So if we use uninflated text sizes to determine intrinsic widths, then
> inflate, doesn't that mean a float or table cell could easily end up too
> small for its text content? Seems like maybe a container's inflation factor
> should have an upper bound of the container width divided by its
> intrinsic-min-width?

The whole point of the complexity here is that the inflation is not affecting intrinsic widths, for exactly that reason.
Suppose I have a block which is sized at its intrinsic-min-width ... say it has a 100-character word, 5px per character, so 500px wide. The user zooms that block to fill the screen. Say the screen is 250px wide. So we inflate the font size by some amount. Now that word doesn't fit, it'll overflow.

Maybe that's OK; I guess it can only happen when the content that needs to go on one line is just too long to be readable and fit on the screen, in which case letting it inflate to overflow is reasonable.
(Assignee)

Comment 77

6 years ago
Forgot to mention earlier:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dbaron@mozilla.com-e0e833c5f6b7/try-android/
has builds based on the attached patches (on top of
https://hg.mozilla.org/mozilla-central/rev/767693e248aa ).
(Assignee)

Comment 78

6 years ago
Responses to all of roc's review comments:

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #68)
> Comment on attachment 569862 [details] [diff] [review] [diff] [details] [review]
> Implement computation of font size inflation for improved readibility of
> text on mobile devices.  (, patch 4)
> 
> Review of attachment 569862 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/base/nsLayoutUtils.cpp
> @@ +4413,5 @@
> > +    byLine = aContainerWidth / sFontSizeInflationEmPerLine;
> > +  }
> > +  if (sFontSizeInflationMinTwips != 0) {
> > +    // REVIEW: Is this giving us app units and sizes *not* counting
> > +    // viewport scaling?
> 
> Yes. You probably should include nsIPresShell::GetResolution here.

I'm not sure if I should -- when does that get set to something other than 1.0?  In particular, if it gets set during panning/zooming on mobile, I don't want it.

> There's also a question of what, if anything, we should do about CSS
> transforms. I think probably content inside a CSS transform should not get
> font-size inflation.

Seems reasonable.  Do we have an efficient way to find out if a frame or one of its ancestors has a transform applied, or should I add a new one?  (If so, do you think it's a bad idea to turn the state bit I added into something inflation-specific, or is it worth having two bits that might have use elsewhere?)

> @@ +4431,5 @@
> > +// size of the same text; the aStyleFontSize parameter here should
> > +// always be a font size, and never a line height.
> > +/* static */ float
> > +nsLayoutUtils::FontSizeInflationInner(const nsIFrame *aFrame,
> > +                                      nscoord aMinFontSize)
> 
> No aStyleFontSize parameter here...

Oops; that was from an earlier version; I'll remove the comment.

> @@ +4491,5 @@
> > +   * count as a container, so that, for example, an indented quotation
> > +   * didn't end up with a smaller font size.  However, it's hard to
> > +   * distinguish these situations where we really do want the indented
> > +   * thing to count as a container, so we don't try, and blocks are
> > +   * always containers.
> 
> It seems to me that the ideal thing here would be to have anything that's
> not in a line-of-inlines be a container for font-size inflation. What do you
> think?

I think that's essentially what I've done -- although inline-blocks at similar things (but not form controls) establish their own containers.  What change were you suggesting?


> ::: modules/libpref/src/init/all.js
> @@ +1555,5 @@
> > + * A value greater than zero enables font size inflation for
> > + * pan-and-zoom UIs, so that if a block's width is scaled to match the
> > + * device's width, the fonts in a block are at least the font size
> > + * given.  The value given is in twips, i.e., 1/20 of a point, or 1/1440
> > + * of an inch.
> 
> I'm not sure twips is the right measure here. Why did you choose it? Seems
> to me we should use something that corresponds closer to viewing angle ...
> CSS pixels (modulo scaling)?

It's possible that some way to attempt to measure viewing angle would make sense at the very small end of devices, but I think beyond that devices are likely to be held at a comfortable arm's length -- I really don't think we should be assuming that a user will be holding a 8" tablet at twice the distance as a 4" phone -- I think it's far more likely they'll be close to the same distance.

I chose twips rather than points because I thought people might want fractions of a point, and we don't have float preferences.


(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #71)
> Comment on attachment 569873 [details] [diff] [review] [diff] [details] [review]
> Make other users of font metrics (other than MathML and XUL) honor font size
> inflation.  (, patch 15)
> 
> Review of attachment 569873 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/generic/nsImageFrame.cpp
> @@ +974,5 @@
> >    // Set font and color
> >    aRenderingContext.SetColor(GetStyleColor()->mColor);
> >    nsRefPtr<nsFontMetrics> fm;
> > +  nsLayoutUtils::GetFontMetricsForFrame(this, getter_AddRefs(fm),
> > +    nsLayoutUtils::FontSizeInflationFor(this));
> 
> Maybe FontSizeInflationFor(nsIFrame*) should assert that the frame isn't
> being reflowed?

It does, with some exceptions for XUL boxes (where that's ok, since size is set at the start of reflow).
(Assignee)

Comment 79

6 years ago
(In reply to Boris Zbarsky (:bz) from comment #74)
> Comment on attachment 569863 [details] [diff] [review] [diff] [details] [review]
> When font size inflation is enabled, horizontal resizes of blocks must cause
> a full dirty reflow.  (, patch 5)
> 
> >other than InitResizeFlags and nsFrame::Box
> 
> nsFrame::BoxReflow.

right

> >down to the cell, and once we hit the cell we'll set the code we've
> 
> "we'll hit the code"?  Or some other verb?  I have no idea what "set" means
> there.

Yeah.  Or maybe "once we *reach* the cell we'll *hit* the code we've added here".

> The code changes seem ok, but can the text-size-adjust style lead to
> sitations where font size inflation is enabled on the prescontext but the
> width change doesn't trigger it?  If so, maybe a followup bug on detecting
> those cases and skipping the dirty reflow in them?

I don't immediately see how, since there could be a descendant with the opposite text-size-adjust style that does have inflation.
(Assignee)

Comment 80

6 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #76)
> Suppose I have a block which is sized at its intrinsic-min-width ... say it
> has a 100-character word, 5px per character, so 500px wide. The user zooms
> that block to fill the screen. Say the screen is 250px wide. So we inflate
> the font size by some amount. Now that word doesn't fit, it'll overflow.
> 
> Maybe that's OK; I guess it can only happen when the content that needs to
> go on one line is just too long to be readable and fit on the screen, in
> which case letting it inflate to overflow is reasonable.

I think it's ok; I also don't see an alternative.  If we want inflation to be a function of width, then width can't be a function of inflation.
> Or maybe "once we *reach* the cell we'll *hit* the code we've added here".

Sounds good.
(In reply to David Baron [:dbaron] from comment #78)
> I'm not sure if I should -- when does that get set to something other than
> 1.0?  In particular, if it gets set during panning/zooming on mobile, I
> don't want it.

It gets set during zooming, but don't you want the "visual" font-size here, which is affected by zooming?

> > There's also a question of what, if anything, we should do about CSS
> > transforms. I think probably content inside a CSS transform should not get
> > font-size inflation.
> 
> Seems reasonable.  Do we have an efficient way to find out if a frame or one
> of its ancestors has a transform applied, or should I add a new one?  (If
> so, do you think it's a bad idea to turn the state bit I added into
> something inflation-specific, or is it worth having two bits that might have
> use elsewhere?)

I think you should probably add a new state bit.

You can probably ignore translation-only transforms.

> > @@ +4491,5 @@
> > > +   * count as a container, so that, for example, an indented quotation
> > > +   * didn't end up with a smaller font size.  However, it's hard to
> > > +   * distinguish these situations where we really do want the indented
> > > +   * thing to count as a container, so we don't try, and blocks are
> > > +   * always containers.bringing
> > 
> > It seems to me that the ideal thing here would be to have anything that's
> > not in a line-of-inlines be a container for font-size inflation. What do you
> > think?
> 
> I think that's essentially what I've done -- although inline-blocks at
> similar things (but not form controls) establish their own containers.  What
> change were you suggesting?

I think I was suggesting inline-block, inline-box and inline-table should not establish their own containers. Having them behave inconsistently with form elements makes me a little uncomfortable. Can you outline your reasoning for why they should establish their own containers?

> > ::: modules/libpref/src/init/all.js
> > @@ +1555,5 @@
> > > + * A value greater than zero enables font size inflation for
> > > + * pan-and-zoom UIs, so that if a block's width is scaled to match the
> > > + * device's width, the fonts in a block are at least the font size
> > > + * given.  The value given is in twips, i.e., 1/20 of a point, or 1/1440
> > > + * of an inch.
> > 
> > I'm not sure twips is the right measure here. Why did you choose it? Seems
> > to me we should use something that corresponds closer to viewing angle ...
> > CSS pixels (modulo scaling)?
> 
> It's possible that some way to attempt to measure viewing angle would make
> sense at the very small end of devices, but I think beyond that devices are
> likely to be held at a comfortable arm's length -- I really don't think we
> should be assuming that a user will be holding a 8" tablet at twice the
> distance as a 4" phone -- I think it's far more likely they'll be close to
> the same distance.

I don't mean viewing angle of the entire screen, but angle subtended at the eye by the unit. CSS px is approximately a constant angle subtended at the eye, as you know. So if we want text to appear to be at least a certain size, CSS px seems like a suitable unit.
(In reply to David Baron [:dbaron] from comment #80)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #76)
> > Suppose I have a block which is sized at its intrinsic-min-width ... say it
> > has a 100-character word, 5px per character, so 500px wide. The user zooms
> > that block to fill the screen. Say the screen is 250px wide. So we inflate
> > the font size by some amount. Now that word doesn't fit, it'll overflow.
> > 
> > Maybe that's OK; I guess it can only happen when the content that needs to
> > go on one line is just too long to be readable and fit on the screen, in
> > which case letting it inflate to overflow is reasonable.
> 
> I think it's ok; I also don't see an alternative.  If we want inflation to
> be a function of width, then width can't be a function of inflation.

A possible alternative would be to choose the inflation factor so that the intrinsic-min-width multiplied by the inflation factor doesn't exceed the container width.

I think we probably shouldn't worry about this, though.
Attachment #515559 - Attachment is obsolete: true
Attachment #515507 - Attachment is obsolete: true
Attachment #515638 - Attachment is obsolete: true
(Assignee)

Comment 84

6 years ago
(In reply to David Baron [:dbaron] from comment #78)
> > There's also a question of what, if anything, we should do about CSS
> > transforms. I think probably content inside a CSS transform should not get
> > font-size inflation.
> 
> Seems reasonable.  Do we have an efficient way to find out if a frame or one
> of its ancestors has a transform applied, or should I add a new one?  (If
> so, do you think it's a bad idea to turn the state bit I added into
> something inflation-specific, or is it worth having two bits that might have
> use elsewhere?)

Actually, now that I've implemented this, it's a little less reasonable, since the inflation is per-block, and CSS transforms can be on an inline.  But I suppose I'll live with:  inflation will be disabled if the block is transformed, but for a transform on an inline, inflation will still apply.  Or should I scratch the whole idea instead, or try to change things so that we don't assume it doesn't vary within a block (I'd need to go back and look at what depends on that)?
(In reply to David Baron [:dbaron] from comment #84)
> Actually, now that I've implemented this, it's a little less reasonable,
> since the inflation is per-block, and CSS transforms can be on an inline. 
> But I suppose I'll live with:  inflation will be disabled if the block is
> transformed, but for a transform on an inline, inflation will still apply.

I think that's totally fine.
(Assignee)

Comment 86

6 years ago
I've debugged most of the test failures I've hit, but now I've come to one I'm not sure (immediately, anyway) what to do about.  The failure is the abort that the reflow state chain is consistent with frame parentage (in nsLayoutUtils::InflationMinFontSizeFor(const nsHTMLReflowState &aReflowState)), failing on layout/reftests/bugs/409084-1a.html.  It's failing because there's a codepath in table reflow that chains the cell reflow state directly to a row group reflow state without a row reflow state in between, in particular the call in nsTableRowGroupFrame::SplitSpanningCells() to row->ReflowCellFrame().  (There's a row reflow state constructed earlier in the function, inside a condition that that call isn't inside of.)
(Assignee)

Comment 87

6 years ago
(In reply to David Baron [:dbaron] from comment #86)
> (There's a row reflow state constructed earlier in the function, inside a
> condition that that call isn't inside of.)

Er, sorry, there's a row reflow state constructed earlier in nsTableRowGroupFrame::SplitRowGroup, which is the caller of SplitSpanningCells, but inside a condition not containing the call to SplitSpanningCells.
Just make the row reflow state construction unconditional, and pass it down? Seems like the logical thing to do.
(Assignee)

Comment 89

6 years ago
Created attachment 572726 [details] [diff] [review]
Don't duplicate frame state bits, so that we can assert about NS_FRAME_IN_REFLOW during painting.  (, patch 3.5)
(Assignee)

Comment 90

6 years ago
Created attachment 572727 [details] [diff] [review]
Add the NS_FRAME_INSIDE_TRANSFORM for frames that are inside of a CSS transform.  (, patch 3.75)
(Assignee)

Comment 91

6 years ago
Created attachment 572728 [details] [diff] [review]
Don't construct cell reflow states with a row group reflow state as their parent; instead, always link in a table row reflow state as appropriate.  (, patch 3.875)
(Assignee)

Comment 92

6 years ago
Created attachment 572729 [details] [diff] [review]
Implement computation of font size inflation for improved readibility of text on mobile devices.  (, patch 4)
(Assignee)

Comment 93

6 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #88)
> Just make the row reflow state construction unconditional, and pass it down?
> Seems like the logical thing to do.

Yeah, I was worried about the performance of doing that, but now that I've looked at the code more, I'm not.  (We'd still only construct the reflow state for a row that we're trying to split or push.)
(Assignee)

Updated

6 years ago
Attachment #569862 - Attachment is obsolete: true
Attachment #569862 - Flags: review?(roc)
(Assignee)

Comment 94

6 years ago
Created attachment 572730 [details] [diff] [review]
Run existing tests without font size inflation, even when it is enabled.  (, patch 17)
(Assignee)

Updated

6 years ago
Attachment #572727 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #572728 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #572729 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #572730 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Attachment #569865 - Flags: review?(matspal) → review?(roc)
Attachment #572727 - Flags: review?(roc) → review+
Attachment #572728 - Flags: review?(roc) → review+
Attachment #569865 - Flags: review?(roc) → review+
David, I think you didn't respond to comment #69?
Attachment #572730 - Flags: review?(roc) → review+
Parts of comment #82 as well.
(Assignee)

Comment 97

6 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #95)
> David, I think you didn't respond to comment #69?

Right; I'm still working on updating patch 9.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #96)
> Parts of comment #82 as well.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #82)
> (In reply to David Baron [:dbaron] from comment #78)
> > I'm not sure if I should -- when does that get set to something other than
> > 1.0?  In particular, if it gets set during panning/zooming on mobile, I
> > don't want it.
> 
> It gets set during zooming, but don't you want the "visual" font-size here,
> which is affected by zooming?

What sort of zooming are you talking about?  I absolutely do *not* want this to be affected by the zooming/panning that happens on mobile -- the whole point is choosing a font size so that after the user zooms a block to fill the width of the device, the text is large enough to be readable -- and to not break the zoom/pan model, i.e., apply the inflation to all zooms.

> > > There's also a question of what, if anything, we should do about CSS
> > > transforms. I think probably content inside a CSS transform should not get
> > > font-size inflation.
> > 
> > Seems reasonable.  Do we have an efficient way to find out if a frame or one
> > of its ancestors has a transform applied, or should I add a new one?  (If
> > so, do you think it's a bad idea to turn the state bit I added into
> > something inflation-specific, or is it worth having two bits that might have
> > use elsewhere?)
> 
> I think you should probably add a new state bit.
> 
> You can probably ignore translation-only transforms.

This is done in the revised patch 4.

> > > @@ +4491,5 @@
> > > > +   * count as a container, so that, for example, an indented quotation
> > > > +   * didn't end up with a smaller font size.  However, it's hard to
> > > > +   * distinguish these situations where we really do want the indented
> > > > +   * thing to count as a container, so we don't try, and blocks are
> > > > +   * always containers.bringing
> > > 
> > > It seems to me that the ideal thing here would be to have anything that's
> > > not in a line-of-inlines be a container for font-size inflation. What do you
> > > think?
> > 
> > I think that's essentially what I've done -- although inline-blocks at
> > similar things (but not form controls) establish their own containers.  What
> > change were you suggesting?
> 
> I think I was suggesting inline-block, inline-box and inline-table should
> not establish their own containers. Having them behave inconsistently with
> form elements makes me a little uncomfortable. Can you outline your
> reasoning for why they should establish their own containers?

I could go either way on this, I suppose, but basically because I think inline-block tends to be used in ways that are only "inline" in a layout sense, not a conceptual sense, such as a bunch of inline blocks all in a row with no other inline content.

> > > ::: modules/libpref/src/init/all.js
> > > @@ +1555,5 @@
> > > > + * A value greater than zero enables font size inflation for
> > > > + * pan-and-zoom UIs, so that if a block's width is scaled to match the
> > > > + * device's width, the fonts in a block are at least the font size
> > > > + * given.  The value given is in twips, i.e., 1/20 of a point, or 1/1440
> > > > + * of an inch.
> > > 
> > > I'm not sure twips is the right measure here. Why did you choose it? Seems
> > > to me we should use something that corresponds closer to viewing angle ...
> > > CSS pixels (modulo scaling)?
> > 
> > It's possible that some way to attempt to measure viewing angle would make
> > sense at the very small end of devices, but I think beyond that devices are
> > likely to be held at a comfortable arm's length -- I really don't think we
> > should be assuming that a user will be holding a 8" tablet at twice the
> > distance as a 4" phone -- I think it's far more likely they'll be close to
> > the same distance.
> 
> I don't mean viewing angle of the entire screen, but angle subtended at the
> eye by the unit. CSS px is approximately a constant angle subtended at the
> eye, as you know. So if we want text to appear to be at least a certain
> size, CSS px seems like a suitable unit.

I have no idea what you're proposing here.
> > I think you should probably add a new state bit.
> > 
> > You can probably ignore translation-only transforms.
>
> This is done in the revised patch 4.

You're not ignoring translation-only transforms in patch 3.75.

There's another issue involving subdocuments. Right now you're not propagating the has-transform flag down into subdocuments. We could fix that, but in bug 699351 it looks like mobile devs are going to implement mobile zooming using transforms on the <browser> element, and we don't want to disable font-size inflation in there.

So maybe we should just ignore transforms after all.

> I have no idea what you're proposing here.

I think ideally we want the "minimum visual font size" to be specified in a unit that's proportional to the angle subtended at the eye, rather than a physical unit. A physical unit value that's a good minimum font size at one viewing distance will be incorrect for another viewing distance.

When no zooming of any kind is present, CSS pixels are a reasonable unit. However, I don't see anything in our code that actually ensures the device-pixel-to-CSS-pixel ratio is set correctly on mobile. So let's leave this alone for now. If we decide later that a viewing-angle-based pref is necessary, we can add it as an extra pref.

> I could go either way on this

Me too, so let's not change it.
(Assignee)

Comment 99

6 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #98)
> > > I think you should probably add a new state bit.
> > > 
> > > You can probably ignore translation-only transforms.
> >
> > This is done in the revised patch 4.
> 
> You're not ignoring translation-only transforms in patch 3.75.

Oops, yes, I missed the last line there.

Ignoring translation-only transforms is hard, though, since changing a transform from translation-only to non-translation-only need not trigger reflow.  If we wanted this, we'd need to make it trigger reflow again, which I really don't want to do.  So I think that's the wrong thing to do.

> There's another issue involving subdocuments. Right now you're not
> propagating the has-transform flag down into subdocuments. We could fix
> that, but in bug 699351 it looks like mobile devs are going to implement
> mobile zooming using transforms on the <browser> element, and we don't want
> to disable font-size inflation in there.
> 
> So maybe we should just ignore transforms after all.

Yeah, I think that's probably best.

I think it makes sense to stop doing inflation when there are transforms *between* the containing block for the inflation and the element being transformed.  However, that's exactly the opposite of what I described in comment 84, so I think what I've done for ignoring transforms is entirely useless.

So I'll pull patch 3.75 and revise patch 4 again to match.

> > I have no idea what you're proposing here.
> 
> I think ideally we want the "minimum visual font size" to be specified in a
> unit that's proportional to the angle subtended at the eye, rather than a
> physical unit. A physical unit value that's a good minimum font size at one
> viewing distance will be incorrect for another viewing distance.
> 
> When no zooming of any kind is present, CSS pixels are a reasonable unit.
> However, I don't see anything in our code that actually ensures the
> device-pixel-to-CSS-pixel ratio is set correctly on mobile. So let's leave
> this alone for now. If we decide later that a viewing-angle-based pref is
> necessary, we can add it as an extra pref.

ok.  (I'm still not sure how you're proposing we figure out how far the device is from the user's eyes.)
> However, I don't see anything in our code that actually ensures the device-pixel
> -to-CSS-pixel ratio is set correctly on mobile.

There is browser.viewport.scaleRatio, but it only feeds into calculations in getScaleRatio() in browser.js, and there's no way to access that directly from layout of course. We probably should fix that, but not here.
(In reply to David Baron [:dbaron] from comment #99)
> ok.  (I'm still not sure how you're proposing we figure out how far the
> device is from the user's eyes.)

With the sensors on some devices these days, we could directly detect it!

Short of that, I think we'd hardcode an estimate for particular device types or use system settings. We need to do this for CSS px to work well across devices anyway.
(Assignee)

Updated

6 years ago
Attachment #572727 - Attachment is obsolete: true
(Assignee)

Comment 102

6 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #69)
> @@ +417,5 @@
> > +  void ClearTextRun(nsTextFrame* aStartContinuation, bool aInflated);
> > +
> > +  void ClearTextRuns() {
> > +    ClearTextRun(nsnull, true);
> > +    ClearTextRun(nsnull, false);
> 
> Pass a mask to ClearTextRun so we don't have to do two passes?

Looking at ClearTextRun, this doesn't seem so productive, since what ClearTextRun does is get the text run and do a bunch of things with it.  It seems far less invasive to just call it twice, unless I'm missing some other way to do this.
(Assignee)

Updated

6 years ago
Attachment #572726 - Flags: review?(roc)
(Assignee)

Comment 103

6 years ago
Created attachment 574212 [details] [diff] [review]
Don't construct cell reflow states with a row group reflow state as their parent; instead, always link in a table row reflow state as appropriate.  (, patch 3.875)
(Assignee)

Updated

6 years ago
Attachment #572728 - Attachment is obsolete: true
(Assignee)

Comment 104

6 years ago
Created attachment 574214 [details] [diff] [review]
Apply font size inflation to text.  (, patch 9)
(Assignee)

Updated

6 years ago
Attachment #569867 - Attachment is obsolete: true
Attachment #569867 - Flags: review?(roc)
(Assignee)

Comment 105

6 years ago
Created attachment 574215 [details] [diff] [review]
Make other users of font metrics (other than MathML and XUL) honor font size inflation.  (, patch 15)
(Assignee)

Updated

6 years ago
Attachment #569873 - Attachment is obsolete: true
(Assignee)

Comment 106

6 years ago
Comment on attachment 574212 [details] [diff] [review]
Don't construct cell reflow states with a row group reflow state as their parent; instead, always link in a table row reflow state as appropriate.  (, patch 3.875)

The previous version of this patch was actually wrong, since it was constructing the reflow state for the wrong row.  This is just the simple approach of constructing a new reflow state right where we need it, which I think is the best we can do.
Attachment #574212 - Flags: review?(roc)
(Assignee)

Comment 107

6 years ago
Comment on attachment 574214 [details] [diff] [review]
Apply font size inflation to text.  (, patch 9)

This is revised per your previous comments (see also comment 102).
Attachment #574214 - Flags: review?(roc)
(Assignee)

Comment 108

6 years ago
Comment on attachment 574215 [details] [diff] [review]
Make other users of font metrics (other than MathML and XUL) honor font size inflation.  (, patch 15)

You reviewed this one before, but I needed to fix up the nsListControlFrame changes substantially, so they need re-review.  (The changes in the old patch were trivial; the new ones are less so.)
Attachment #574215 - Flags: review?(roc)
Attachment #572726 - Flags: review?(roc) → review+
Attachment #574212 - Flags: review?(roc) → review+
Attachment #574215 - Flags: review?(roc) → review+
Attachment #574214 - Flags: review?(roc) → review+
Comment on attachment 574214 [details] [diff] [review]
Apply font size inflation to text.  (, patch 9)

Review of attachment 574214 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsTextFrameThebes.cpp
@@ +4104,5 @@
> +    return true;
> +  }
> +  FrameProperties props = Properties();
> +  if (props.Get(UninflatedTextRunProperty()) == aTextRun) {
> +    props.Delete(UninflatedTextRunProperty());

Can we avoid this Get() by checking TEXT_HAS_FONT_INFLATION first? I think so, so let's.
(Assignee)

Comment 110

6 years ago
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #109)
> Comment on attachment 574214 [details] [diff] [review] [diff] [details] [review]
> Apply font size inflation to text.  (, patch 9)
> 
> Review of attachment 574214 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/generic/nsTextFrameThebes.cpp
> @@ +4104,5 @@
> > +    return true;
> > +  }
> > +  FrameProperties props = Properties();
> > +  if (props.Get(UninflatedTextRunProperty()) == aTextRun) {
> > +    props.Delete(UninflatedTextRunProperty());
> 
> Can we avoid this Get() by checking TEXT_HAS_FONT_INFLATION first? I think
> so, so let's.

I'll try it.  This code was definitely fragile, and I no longer remember all of what I knew about it when I wrote this.

Comment 111

6 years ago
Try run for beb6bb1a500f is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=beb6bb1a500f
Results (out of 277 total builds):
    success: 253
    warnings: 24
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dbaron@mozilla.com-beb6bb1a500f
(Assignee)

Comment 112

6 years ago
I've made 2 minor changes to diagnose and fix test failures that I don't feel need re-review:

 (1) I changed patch 16 to output diagnostics for reftest failures in a format that
     reftest-analyzer can read (and put the function used to do that in WindowSnapshot.js)

 (2) I changed the following in patch 6:
     +  font.size *= aInflation;
     into:
     +  font.size = NSToCoordRound(font.size * aInflation);
     to avoid letting floating point error load to picking the wrong nscoord when there's an
     exactly correct nscoord.
Attachment #572729 - Flags: review?(roc) → review+
(Assignee)

Comment 113

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9abc8ef4626
https://hg.mozilla.org/integration/mozilla-inbound/rev/d62512892555
https://hg.mozilla.org/integration/mozilla-inbound/rev/69f89ee5d08f
https://hg.mozilla.org/integration/mozilla-inbound/rev/c511af7d8f58
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a6a560a1492
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac0ec1183d19
https://hg.mozilla.org/integration/mozilla-inbound/rev/46669afabd15
https://hg.mozilla.org/integration/mozilla-inbound/rev/74f32abaa8c0
https://hg.mozilla.org/integration/mozilla-inbound/rev/f197554cf989
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a2405eb5b90
https://hg.mozilla.org/integration/mozilla-inbound/rev/b48954598d7d
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0a82577259c
https://hg.mozilla.org/integration/mozilla-inbound/rev/0235d1541e58
https://hg.mozilla.org/integration/mozilla-inbound/rev/450f2557d3a2
https://hg.mozilla.org/integration/mozilla-inbound/rev/e24d196602bf
https://hg.mozilla.org/integration/mozilla-inbound/rev/03c2ea0eeba3
https://hg.mozilla.org/integration/mozilla-inbound/rev/7aca4ef8e538
https://hg.mozilla.org/integration/mozilla-inbound/rev/11ec362e780b
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b92a3b96446

Need followups for:
 * XUL and MathML
 * further testing of the size detection code across devices
 * figuring out UI for the prefs, if any
 * settling on default values for the prefs
 * the FIXME/REVIEW remaining in the landed patches
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/7c7dc8193692

https://tbpl.mozilla.org/php/getParsedLog.php?id=7395819&tree=Mozilla-Inbound for Windows crashtest assertions in 478131-1.html, https://tbpl.mozilla.org/php/getParsedLog.php?id=7396505&tree=Mozilla-Inbound and https://tbpl.mozilla.org/php/getParsedLog.php?id=7395344&tree=Mozilla-Inbound for failures in both halves of Android reftests. (Outside chance it also turns the frequent timeouts in Android crashtests into permanent timeouts in C1, retriggers for that are still going.)
Come to think of it, I don't remember ever seeing a crashtest on Android clearly failing, probably the "2400 seconds without output" during 446328.html four times in a row is its charming way of saying that it crashed.
Blocks: 703029

Comment 116

6 years ago
Try run for 541a6a13580c is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=541a6a13580c
Results (out of 289 total builds):
    exception: 11
    success: 163
    warnings: 19
    failure: 96
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dbaron@mozilla.com-541a6a13580c
(Assignee)

Comment 117

6 years ago
I fixed the Windows crashtest assertion with the added aInflation != 1.0f test in https://hg.mozilla.org/mozilla-central/rev/e233695b1310 ; the problem was that round-tripping large numbers through floats was lossy.

Fixed the Android R3 and C1 problems when I finally realized that my problem was that I hadn't fully disabled inflation while running reftests, since I additionally needed to disable it in remotereftest.py.  See the second file's worth of changes in https://hg.mozilla.org/mozilla-central/rev/cf764be32bc3

Relanded:
https://hg.mozilla.org/mozilla-central/rev/ada794061c14
https://hg.mozilla.org/mozilla-central/rev/1aaacbcb3b61
https://hg.mozilla.org/mozilla-central/rev/4751e5b548a7
https://hg.mozilla.org/mozilla-central/rev/e229fd69d8b1
https://hg.mozilla.org/mozilla-central/rev/3d486b1ad76a
https://hg.mozilla.org/mozilla-central/rev/d3e230bec2f9
https://hg.mozilla.org/mozilla-central/rev/a10a1bc3cb40
https://hg.mozilla.org/mozilla-central/rev/e233695b1310
https://hg.mozilla.org/mozilla-central/rev/b0eb968eceb5
https://hg.mozilla.org/mozilla-central/rev/2a00ab5642c1
https://hg.mozilla.org/mozilla-central/rev/236ba3dfe647
https://hg.mozilla.org/mozilla-central/rev/d090012f8439
https://hg.mozilla.org/mozilla-central/rev/d3724c31c3a0
https://hg.mozilla.org/mozilla-central/rev/738725e75362
https://hg.mozilla.org/mozilla-central/rev/51ebc0da58ba
https://hg.mozilla.org/mozilla-central/rev/ff09e8e9bd8a
https://hg.mozilla.org/mozilla-central/rev/ccabd715b232
https://hg.mozilla.org/mozilla-central/rev/eb173d230150
https://hg.mozilla.org/mozilla-central/rev/cf764be32bc3

I should probably get to the point where we can run crashtests and reftests with inflation enabled.

The reftest failures are all "expected" issues:  in most cases, rounding issues resulting from inflation to a non-integer-pixel font size.  We can fix these with adjustments to the tests (in most cases, -moz-text-size-adjust: none, though in the case of auto-offset-inline-block-1-ref.html and perhaps a few others, by splitting elements in a way that text run splits will match between test and reference).

The crashtest failure is more worrying:  it's probably a sign of something wrong with the patch, but it's really hard to figure out what, and I think I've got a better chance of figuring it out once the patch is landed since then I'll be getting crash data.  The basic problem with crashtests is that a crashtest run that includes all three of layout/base/, layout/forms/, and layout/generic/ leads to the crashtest run disconnecting.  (Perhaps due to a crash?  Android crashtest/reftest runs have pretty much useless diagnostics.)  But if I run any one of those directories alone, it doesn't crash.  So it's not clear how to isolate the problem to a useful testcase, and there's no way to get a crash stack or anything close out of tinderbox (since it just reports the issue as a disconnected tegra).


Additional followup (in addition to comment 113 and the above):
  It's turned on by default in mobile/xul/app/mobile.js .  But that probably doesn't apply to the native UI, so I'd need to write something doing that and land it on birch (after the rest of this merges to birch, presumably).
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Priority: -- → P3
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
(Assignee)

Comment 118

6 years ago
I merged mozilla-central to birch, and then pushed the same default pref patch that I'd pushed for the XUL UI for the Android UI as well:
https://hg.mozilla.org/projects/birch/rev/ce03d0240a30

Updated

6 years ago
Depends on: 705258

Updated

6 years ago
Depends on: 705278

Updated

6 years ago
Depends on: 705287
No longer depends on: 705258
No longer depends on: 705287
(Assignee)

Updated

6 years ago
Depends on: 705446
(Assignee)

Comment 119

6 years ago
One more followup, from today's Platform meeting:
[2011-11-29 11:08:17] <mounir> dbaron: button elements are small and that's weird :(

Comment 120

6 years ago
At least "-moz-text-size-adjust" needs documentation.
http://dbaron.org/log/20111126-font-inflation
Keywords: dev-doc-needed

Updated

6 years ago
Depends on: 706889

Updated

6 years ago
Depends on: 707361
(Assignee)

Updated

6 years ago
Depends on: 705546
(Assignee)

Updated

6 years ago
Depends on: 707195
(Assignee)

Updated

6 years ago
Depends on: 706765
(Assignee)

Updated

6 years ago
Depends on: 706609
(Assignee)

Updated

6 years ago
Depends on: 706198
(Assignee)

Updated

6 years ago
Depends on: 706193
(Assignee)

Updated

6 years ago
Depends on: 706151
(Assignee)

Updated

6 years ago
Depends on: 708036
(Assignee)

Updated

6 years ago
Depends on: 707855
(Assignee)

Updated

6 years ago
Depends on: 707725
(Assignee)

Updated

6 years ago
Depends on: 708187
(Assignee)

Updated

6 years ago
Alias: font-inflation
(Assignee)

Updated

6 years ago
Depends on: 708175
(Assignee)

Updated

6 years ago
Depends on: 708745

Updated

6 years ago
Depends on: 709536
(Assignee)

Updated

6 years ago
Depends on: 707557
(Assignee)

Updated

6 years ago
Depends on: 710137
Depends on: 710517
(Assignee)

Updated

6 years ago
Depends on: 710808
(Assignee)

Updated

6 years ago
Depends on: 709651
Depends on: 711418

Updated

6 years ago
Depends on: 711755
Blocks: 711744
Blocks: 711759

Updated

6 years ago
Depends on: 712708
(Assignee)

Updated

6 years ago
No longer blocks: 711759
Depends on: 711759
Depends on: 713241

Updated

6 years ago
Depends on: 714080
Depends on: 713524

Updated

6 years ago
Blocks: 713635

Updated

6 years ago
Depends on: 716239
No longer depends on: 716239

Updated

6 years ago
Depends on: 716882
Blocks: 715179
(Assignee)

Updated

6 years ago
Depends on: 718290

Updated

6 years ago
Depends on: 719142
(Assignee)

Updated

6 years ago
Depends on: 719378
Depends on: 720567
Depends on: 716725

Updated

6 years ago
Depends on: 721126
Depends on: 721301

Updated

6 years ago
Depends on: 722700

Updated

6 years ago
Depends on: 723947

Updated

6 years ago
Depends on: 723965
Depends on: 724827

Updated

6 years ago
Depends on: 724857

Updated

5 years ago
Depends on: 728560
Depends on: 707917
Depends on: 729961

Updated

5 years ago
Depends on: 719685

Updated

5 years ago
Depends on: 732284

Updated

5 years ago
Depends on: 732290
Depends on: 732539
Depends on: 705657
I've documented this (as well as MS and Webkit behavior, as far as I was able to collect the information): https://developer.mozilla.org/en/CSS/text-size-adjust

Two questions: 
1) MS ignore -ms-text-size-adjust if the viewport is set via <meta>. This is not the case of Gecko, is that right?
2) Will this be in Firefox 11 for mobile? (Should I update Firefox 11 for developers?)
Keywords: dev-doc-needed → dev-doc-complete
This will ship in the first Firefox mobile native release. Currently that is targeting Firefox 13.

Updated

5 years ago
Depends on: 735149
Depends on: 733169

Updated

5 years ago
Depends on: 735609

Updated

5 years ago
Depends on: 737270

Updated

5 years ago
Depends on: 737872

Updated

5 years ago
Depends on: 738240

Updated

5 years ago
Depends on: 737621

Updated

5 years ago
Depends on: 741419

Updated

5 years ago
Depends on: 744924

Updated

5 years ago
No longer depends on: 724857

Updated

5 years ago
No longer depends on: 719142

Updated

5 years ago
Depends on: 746966

Updated

5 years ago
Depends on: 747267

Updated

5 years ago
Depends on: 747720

Updated

5 years ago
Depends on: 747857

Updated

5 years ago
Depends on: 748446

Updated

5 years ago
Depends on: 748699

Updated

5 years ago
Depends on: 750749

Updated

5 years ago
Depends on: 748054

Updated

5 years ago
Depends on: 752317
Depends on: 748398

Updated

5 years ago
Depends on: 748434
Depends on: 753211

Updated

5 years ago
Depends on: 752980
Depends on: 753359

Updated

5 years ago
Depends on: 754122

Updated

5 years ago
Depends on: 754456
(Assignee)

Updated

5 years ago
Depends on: 747231
Depends on: 752029
No longer depends on: 752029
Depends on: 752033
Depends on: 754992
Depends on: 755803
No longer depends on: 752033
(Assignee)

Updated

5 years ago
Depends on: 756292
Depends on: 757259
Depends on: 757257

Updated

5 years ago
Depends on: 757363

Updated

5 years ago
Depends on: 757317
Depends on: 758050
Depends on: 759845

Updated

5 years ago
Depends on: 760098

Updated

5 years ago
Depends on: 760151

Updated

5 years ago
Depends on: 761145

Updated

5 years ago
Depends on: 762372

Updated

5 years ago
Depends on: 763957

Updated

5 years ago
Depends on: 755770

Updated

5 years ago
Depends on: 766170

Updated

5 years ago
Depends on: 766172

Updated

5 years ago
Depends on: 767351
Depends on: 757179
Depends on: 758079

Updated

5 years ago
Depends on: 769175

Updated

5 years ago
Depends on: 769681
Depends on: 769958

Updated

5 years ago
Depends on: 772827

Updated

5 years ago
Blocks: 784375
Depends on: 786382
Depends on: 815657

Updated

5 years ago
No longer depends on: 709536
No longer depends on: 769958

Updated

5 years ago
Depends on: 827219

Updated

4 years ago
Depends on: 835472

Updated

4 years ago
Depends on: 914319

Updated

4 years ago
Depends on: 917397
Depends on: 1002526

Updated

3 years ago
Depends on: 1014260

Updated

3 years ago
Depends on: 1032424
Depends on: 1142461
Blocks: 1339619
You need to log in before you can comment on or make changes to this bug.