Closed Bug 448281 Opened 12 years ago Closed 11 years ago

font-size sometimes gets calculated wrong

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla1.9.1a2

People

(Reporter: syskin2, Assigned: dbaron)

References

()

Details

(Keywords: regression, verified1.9.1)

Attachments

(3 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1a2pre) Gecko/2008072803 Firefox/3.1a2pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1a2pre) Gecko/2008072803 Minefiled/3.1a2pre

Starting with yesterday's nightly, some fonts are *sometimes* too big on some websites. DOM inspector reveals that calculated font-size is indeed incorrect, being too big by a random (but consistent on one website) factor. It happens randomly.

Reproducible: Sometimes

Steps to Reproduce:
1. visit http://forums.whirlpool.net.au/ or http://forums.mozillazine.org/index.php
2. reload it
Actual Results:  
Example what you *might* see on mozillazine: http://members.ii.net/~syskin/fontproblem.png and after reload, you get more familiar http://members.ii.net/~syskin/fontproblem2.png

No font zoom, regular zoom or OS dpi change was made. Fonts are bigger by consistent 60%.

Example from whirlpool:
Looking fine: http://members.ii.net/~syskin/wp2.png
After reload: http://members.ii.net/~syskin/wp1.png

Some fonts are reliably bigger by exactly 25%, others are unaffected. They have nothing obviously in common with regards to font-family. They have identical CSS rules according to DOMi, but different CSS computed style.


Expected Results:  
Consistency :)


Discussion at http://forums.mozillazine.org/viewtopic.php?f=23&t=768225

Whirlpool problem was confirmed on OSX as well.

I have a strong feeling I've seen such problems for ages (long before FF 3) but VERY occasionally. I remember a bugzilla bug being suddenly too large with a reload fixing it.

After saving whirlpool page to disk, I can't reproduce the problem at all. Randomness might be a timing issue.
Confirming based on comments on mozillazine.org
Status: UNCONFIRMED → NEW
Ever confirmed: true
Thanks to mozillazine's smsmith, new better steps to reproduce were found.

Reproducible: Kinda often Not Always

1. Open mozillazine forums index
2. Ensure you have full page zoom selected, and tap ctrl+ OR ctrl- to change zoom away from 100%
3. Reload
4. (optional) change zoom back to 100% by pressing ctrl0

Actual results: 
* Fonts are bigger after step 3 than they were after step 2
* Fonts are bigger after step 4 than they were after step 1

Expected results: fonts are consistent for any fixed zoom level
And now, some fun: do steps above but for this bug page.

Fonts don't change, but observe "Attachments" panel - it grows larger for no reason! :)

So it's probably not just fonts, any random website element might be rendered bigger than it should be.
Regression range 20080726 - no issue
                 20080727 - this issue exists.
Keywords: regression
Flags: wanted1.9.1?
I have verified this a regression from bug 156716 via source pulls by changeset identifier.
Blocks: mediaqueries
In the steps to reproduce it should also say to ensure you do NOT have "View -> Zoom -> Zoom Text Only" selected.
(In reply to comment #5)
> I have verified this a regression from bug 156716 via source pulls by changeset
> identifier.

If you did that, did you happen to notice which changeset?
This should be blocking 1.9.1 due to it being an ugly regression.
Flags: blocking1.9.1?
Version: unspecified → Trunk
(In reply to comment #5)
> I have verified this a regression from bug 156716 via source pulls by changeset
> identifier.

Also, which testcases did you verify were a regression?
Version: Trunk → unspecified
(In reply to comment #9)
> (In reply to comment #5)
> > I have verified this a regression from bug 156716 via source pulls by changeset
> > identifier.
> 
> Also, which testcases did you verify were a regression?
> 
The case of visiting http://forums.mozillazine.org/viewforum.php?f=23, doing a ctrl + to increase the zoom level then doing a reload of the page and seeing some section get much larger on the reload, then doing a ctrl 0 and not having the page return to normal until doing a reload.

Everything worked normally on a clone specifying the changeset immediately prior to when the first code for this bug was pushed, and not so good on a build form an hg clone specifying the last changeset pushed for this bug.

I was planning to try to narrow it down to one of the individual checkins for this bug unless you feel this is not necessary.
I have not been able to reproduce the issue in comment #0, so I cannot speak to a regression bug for that.  The issues raised in comment #2 and comment #3 both appear to have regressed with the checkin of the following changeset:

dbaron
Sat Jul 26 16:15:36 2008 +0000

8050b146979b

Implement Media Queries, part 4: infrastructure for dynamic
change handling at the pres context level. (Bug 156716)
r+sr=bzbarsky
I can confirm Bill's regression range with http://forums.whirlpool.net.au/ linked from comment 0.
STR
1. load site
2. pay attention to the font-size of the subheadings (blue foreground-colour, grey background)
3. reload the page, watch the size of those sub headlines shrink

Now the puzzling thing I found when it was posted in the forums:
a. view source, fetch the url to the stylesheet, paste it in the same tab
b. back-button to the html page
c. the subheadings are bigger again.
I see this frequently on google reader too.
Duplicate of this bug: 449126
We did alot of testing of this in Bug 449126 before we realized the duplicate bug you might read through it. (Screenshots of the bug are in it)

I was experiencing the problem at:
http://www.newegg.com
and the member's pages at http://www.icanhascheezburger.com

Confirmed with 3 setups, one Vista, 2 XP Pro.
Keywords: qawanted
Version: unspecified → Trunk
This reminds me a lot of bug 436717. An indeed, it can be triggered with the testcase in that bug too, when following the steps to reproduce in comment 2 of this bug.
More evidence it is related, the urls mentioned in this bug are all in strict mode.
I guess there is something in the site specific preferences code that triggers it now.
Version: Trunk → unspecified
My gut feeling is that there are two bugs here:

 1) something causing the MQ dynamic change handling to detect that things need to change when they actually don't

 2) an underlying bug (pre-MQ) where bugs like this happen when we rebuild all style data

All the changeset that causes this bug does is cause us to rebuild all style data in additional cases; this normally shouldn't change anything unless styles have actually changed.  See http://hg.mozilla.org/mozilla-central/index.cgi/rev/8050b146979b
And a simplified testcase could be useful here.
It happens for me with https://bugzilla.mozilla.org/attachment.cgi?id=323241 , which is the testcase of bug 436717.
It is perhaps worth pointing out that all affected elements (including width of 'attachments' panel in this bug) have their size specified in em.
(In reply to comment #19)
> It happens for me with https://bugzilla.mozilla.org/attachment.cgi?id=323241 ,
> which is the testcase of bug 436717.

In that testcase, when the bug occurs, the table has rules from quirks.css applied to it (which should not happen).
OK, what's going on is that during the page load process, we first enable the quirk style sheet and then disable it.  (And if we didn't enable it first, it would be enabled by default anyway.)

What's happening here is that we're now triggering the building of the rule cascade before the quirk style sheet is disabled, and then we're not doing anything to clear the cached data once it is disabled:

#0  nsCSSRuleProcessor::RefreshRuleCascade (this=0x91aef0, 
    aPresContext=0x1a722d0)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsCSSRuleProcessor.cpp:2399
#1  0x00007f876dbc5b13 in nsCSSRuleProcessor::MediumFeaturesChanged (
    this=0x91af08, aPresContext=0x1a722d0, aRulesChanged=0x17c0ab0)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsCSSRuleProcessor.cpp:2094
#2  0x00007f876dc10d5c in nsStyleSet::MediumFeaturesChanged (this=0xf18b10, 
    aPresContext=0x1a722d0)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/style/nsStyleSet.cpp:995
#3  0x00007f876dab6021 in nsPresContext::MediaFeatureValuesChanged (
    this=0x1a722d0, aCallerWillRebuildStyleData=1)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/base/nsPresContext.cpp:1454
#4  0x00007f876dab6754 in nsPresContext::SetFullZoom (this=0x1a722d0, 
    aZoom=1.10000002)
    at /home/dbaron/builds/mozilla-central/mozilla/layout/base/nsPresContext.cpp:1168
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Attachment #332496 - Flags: superreview?(bzbarsky)
Attachment #332496 - Flags: review?(bzbarsky)
Attachment #332496 - Attachment description: patch 1: assert if quirk style sheet enabled/disabled after building rule cascade → patch 2: assert if quirk style sheet enabled/disabled after building rule cascade
Attachment #332496 - Flags: superreview?(bzbarsky)
Attachment #332496 - Flags: superreview+
Attachment #332496 - Flags: review?(bzbarsky)
Attachment #332496 - Flags: review+
Comment on attachment 332497 [details] [diff] [review]
patch 1: don't rebuild rule cascades if they're aren't currently any

>+  // early (e.g., before whether we know if the quirk style sheet should

s/if/whether/

r+sr=bzbarsky.

Any sane way to test this?
Attachment #332497 - Flags: superreview?(bzbarsky)
Attachment #332497 - Flags: superreview+
Attachment #332497 - Flags: review?(bzbarsky)
Attachment #332497 - Flags: review+
I landed these:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/ac776b1e0206
http://hg.mozilla.org/mozilla-central/index.cgi/rev/93b1b9aa2831

It's probably possible to test this with chrome privileges, though it's also a good bit of work to write the test... I don't have time right now.


People should test that the various bugs duped to this one are all fixed; I've only looked at Martijn's testcase, since it was simple.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a2
Verified fixed, all's good with all websites mentioned in this bug.
Status: RESOLVED → VERIFIED
Verified fixed here too.
Depends on: 450191
Comment on attachment 332496 [details] [diff] [review]
patch 2: assert if quirk style sheet enabled/disabled after building rule cascade

I can trigger this assertion using the following steps:

1. Turn on SSL warnings.
2. Open an SSL page in a background tab.
3. OK the SSL warning.

The pres shell is trying to change from standards to almost standards mode, so we shouldn't be hitting this code. I don't know if these steps will trigger an assertion when the page really should be in quirks mode.
Attached file Stack for assertion
The assertion happens with a quirks mode page too.
Flags: wanted1.9.1?
Flags: blocking1.9.1?
Flags: blocking1.9.1+
Keywords: fixed1.9.1
Keywords: verified1.9.1
Keywords: fixed1.9.1
Issue is Resolved - removing QA-Wanted Keywords - QA-Wanted query clean-up task
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.