576927 - "ABORT: negative lengths and percents should be rejected by parser" setting textZoom and huge font size together

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Jesse Ruderman]

Attachment: testcase (crashes Firefox when loaded with extension installed)

1. Temporarily install 'DOM Fuzz Lite' from
    https://www.squarefree.com/extensions/domFuzzLite.xpi
2. Load the testcase.

###!!! ABORT: negative lengths and percents should be rejected by parser: 'aFontData.mSize.IsCalcUnit()', file /Users/jruderman/mozilla-central/layout/style/nsRuleNode.cpp, line 2999

#2  0x051138f9 in Abort (aMsg=Could not find the frame base for "Abort(char const*)".
) at /Users/jruderman/mozilla-central/xpcom/base/nsDebugImpl.cpp:379
#3  0x05113e4d in NS_DebugBreak_P (aSeverity=3, aStr=0x54246b4 "negative lengths and percents should be rejected by parser", aExpr=0x5424697 "aFontData.mSize.IsCalcUnit()", aFile=0x5422120 "/Users/jruderman/mozilla-central/layout/style/nsRuleNode.cpp", aLine=2999) at /Users/jruderman/mozilla-central/xpcom/base/nsDebugImpl.cpp:337
#4  0x040aebe4 in nsRuleNode::SetFontSize (aPresContext=0x84edc00, aFontData=@0xbfffc69c, aFont=0x84abf40, aParentFont=0x84abf40, aSize=0x84abf60, aSystemFont=@0xbfffc464, aParentSize=3840, aScriptLevelAdjustedParentSize=3840, aUsedStartStruct=0, aAtRoot=1, aCanStoreInRuleTree=@0xbfffc5bc) at /Users/jruderman/mozilla-central/layout/style/nsRuleNode.cpp:2997
#5  0x040af9c6 in nsRuleNode::SetFont (aPresContext=0x84edc00, aContext=0x84abba0, aMinFontSize=0, aGenericFontID=0 '\0', aFontData=@0xbfffc69c, aParentFont=0x84abf40, aFont=0x84abf40, aUsedStartStruct=0, aCanStoreInRuleTree=@0xbfffc5bc) at /Users/jruderman/mozilla-central/layout/style/nsRuleNode.cpp:3290
#6  0x040b024d in nsRuleNode::ComputeFontData (this=0x8506eb8, aStartStruct=0x0, aData=@0xbfffc69c, aContext=0x84abba0, aHighestNode=0x8506eb8, aRuleDetail=nsRuleNode::eRulePartialReset, aCanStoreInRuleTree=1) at /Users/jruderman/mozilla-central/layout/style/nsRuleNode.cpp:3504
#7  0x040a9882 in nsRuleNode::WalkRuleTree (this=0x8506eb8, aSID=eStyleStruct_Font, aContext=0x84abba0, aRuleData=0xbfffc6f8, aSpecificData=0xbfffc69c) at nsStyleStructList.h:73
#8  0x040add27 in nsRuleNode::GetFontData (this=0x8506eb8, aContext=0x84abba0) at /Users/jruderman/mozilla-central/layout/style/nsRuleNode.cpp:1946
#9  0x040ade23 in nsRuleNode::GetStyleFont (this=0x8506eb8, aContext=0x84abba0, aComputeData=1) at nsStyleStructList.h:73
#10 0x040c5675 in nsStyleContext::DoGetStyleFont (this=0x84abba0, aComputeData=1) at nsStyleStructList.h:73
#11 0x03feba49 in nsStyleContext::GetStyleFont (this=0x84abba0) at nsStyleStructList.h:73
#12 0x040d2fd9 in nsStyleContext::CalcStyleDifference (this=0x84ab770, aOther=0x84abba0) at /Users/jruderman/mozilla-central/layout/style/nsStyleContext.cpp:468
#13 0x03ed47d7 in CaptureChange (aOldContext=0x84ab770, aNewContext=0x84abba0, aFrame=0x84ab950, aContent=0x214057f0, aChangeList=0xbfffcbac, aMinChange=0, aChangeToAssume=0) at /Users/jruderman/mozilla-central/layout/base/nsFrameManager.cpp:978
#14 0x03ed52a7 in nsFrameManager::ReResolveStyleContext (this=0x202fb5cc, aPresContext=0x84edc00, aFrame=0x84ab950, aParentContent=0x0, aChangeList=0xbfffcbac, aMinChange=0, aRestyleHint=eRestyle_Self, aFireAccessibilityEvents=1, aRestyleTracker=@0x202fba44) at /Users/jruderman/mozilla-central/layout/base/nsFrameManager.cpp:1234
#15 0x03ed5ed9 in nsFrameManager::ComputeStyleChangeFor (this=0x202fb5cc, aFrame=0x84ab950, aChangeList=0xbfffcbac, aMinChange=0, aRestyleTracker=@0x202fba44, aRestyleDescendants=0) at /Users/jruderman/mozilla-central/layout/base/nsFrameManager.cpp:1548
#16 0x03e9281b in nsCSSFrameConstructor::RestyleElement (this=0x202fb9e0, aElement=0x214057f0, aPrimaryFrame=0x84ab950, aMinHint=0, aRestyleTracker=@0x202fba44, aRestyleDescendants=0) at /Users/jruderman/mozilla-central/layout/base/nsCSSFrameConstructor.cpp:8121
#17 0x03e78206 in mozilla::css::RestyleTracker::ProcessOneRestyle (this=0x202fba44, aElement=0x214057f0, aRestyleHint=eRestyle_Self, aChangeHint=0) at /Users/jruderman/mozilla-central/layout/base/RestyleTracker.cpp:156
#18 0x03e76ed9 in mozilla::css::RestyleTracker::ProcessRestyles (this=0x202fba44) at /Users/jruderman/mozilla-central/layout/base/RestyleTracker.cpp:238
#19 0x03e925d7 in nsCSSFrameConstructor::ProcessPendingRestyles (this=0x202fb9e0) at /Users/jruderman/mozilla-central/layout/base/nsCSSFrameConstructor.cpp:11687
#20 0x03f0eccc in PresShell::FlushPendingNotifications (this=0x202fb5b0, aType=Flush_Style) at /Users/jruderman/mozilla-central/layout/base/nsPresShell.cpp:4786
#21 0x03f183a2 in nsRefreshDriver::Notify (this=0x202f6d70) at /Users/jruderman/mozilla-central/layout/base/nsRefreshDriver.cpp:221
#22 0x0510a357 in nsTimerImpl::Fire (this=0x21419fe0) at /Users/jruderman/mozilla-central/xpcom/threads/nsTimerImpl.cpp:430
#23 0x0510a574 in nsTimerEvent::Run (this=0x21425750) at /Users/jruderman/mozilla-central/xpcom/threads/nsTimerImpl.cpp:519
#24 0x0510344e in nsThread::ProcessNextEvent (this=0xa16890, mayWait=0, result=0xbfffd884) at /Users/jruderman/mozilla-central/xpcom/threads/nsThread.cpp:547
#25 0x0508bc50 in NS_ProcessPendingEvents_P (thread=0xa16890, timeout=20) at nsThreadUtils.cpp:200
#26 0x04ea3377 in nsBaseAppShell::NativeEventCallback (this=0xa31d40) at /Users/jruderman/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:126
#27 0x04e549d1 in nsAppShell::ProcessGeckoEvents (aInfo=0xa31d40) at /Users/jruderman/mozilla-central/widget/src/cocoa/nsAppShell.mm:394
#28 0x936143c5 in CFRunLoopRunSpecific ()
#29 0x93614aa8 in CFRunLoopRunInMode ()
#30 0x94a9b2ac in RunCurrentEventLoopInMode ()
#31 0x94a9b0c5 in ReceiveNextEventCommon ()
#32 0x94a9af39 in BlockUntilNextEventMatchingListInMode ()
#33 0x95b786d5 in _DPSNextEvent ()
#34 0x95b77f88 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#35 0x95b70f9f in -[NSApplication run] ()
#36 0x04e53783 in nsAppShell::Run (this=0xa31d40) at /Users/jruderman/mozilla-central/widget/src/cocoa/nsAppShell.mm:747
#37 0x04bc28aa in nsAppStartup::Run (this=0xa7ba30) at /Users/jruderman/mozilla-central/toolkit/components/startup/src/nsAppStartup.cpp:192
#38 0x03c2c94d in XRE_main (argc=4, argv=0xbfffedf0, aAppData=0xa0ed20) at /Users/jruderman/mozilla-central/toolkit/xre/nsAppRunner.cpp:3619
#39 0x0000281e in main (argc=4, argv=0xbfffedf0) at /Users/jruderman/mozilla-central/browser/app/nsBrowserApp.cpp:158

Comment 1

[Mats Palmgren (inactive)]

Attachment: fix + mochitest

Comment 2

[Mats Palmgren (inactive)]

To my surprise I see that NSCoordSaturatingMultiply (and
NSCoordSaturatingNonnegativeMultiply) can return negative values
when both arguments are positive... for example
NSCoordSaturatingMultiply(0, 1.0 / 0.0) gives product==-NaN which is
capped to nscoord_MIN. (on Linux64 with gcc 4.4.5 in case it matters)

Comment 3

[Bob Clary [:bc] (inactive)]

I see this also on http://mp3tons.ru/board/ringtony/1-1-2-0-0-2-2-0

I was able to reproduce it locally on 2.0.0, beta, aurora and nightly on winxp without any extension. Automation claims it is cross platform.

Comment 4

[Jesse Ruderman]

STR without the extension:

1. Load https://bugzilla.mozilla.org/attachment.cgi?id=313874 (from bug 427322)
2. View > Zoom > Zoom Text Only
3. Zoom in 6 times using ⌘+

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Is this still relevant?

Comment 6

[Mats Palmgren (inactive)]

Yeah, I can still reproduce the STR in comment 4.
The mochitest is obsolete though, since it uses:
netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect")

Comment 7

[Mats Palmgren (inactive)]

Attachment: fix

https://treeherder.mozilla.org/#/jobs?repo=try&revision=37ffa6d22b29

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cbc8ceae73a2

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for test_value_cloning.html bustage
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e28f202bd55

Comment 10

[Mats Palmgren (inactive)]

This was the offending patch.  It appears we do get negative font sizes (aSize)
in nsStyleFont::ZoomText for things like calc(-1px).  It looks like we evaluate
the terms using ZoomText and then the consumer of the calc expression will
deal with a negative result later, e.g. in nsRuleNode::SetFontSize:

#0  nsStyleFont::ZoomText (aPresContext=0x7fffc5173000, aSize=-60)
#1  SetFontSizeCalcOps::ComputeLeafValue (this=0x7fffffff58e8, aValue=...)
#2  mozilla::css::ComputeCalc<SetFontSizeCalcOps> (aValue=..., aOps=...)
#3  mozilla::css::ComputeCalc<SetFontSizeCalcOps> (aValue=..., aOps=...)
#4  nsRuleNode::SetFontSize

http://hg.mozilla.org/mozilla-central/annotate/4294bf91174b/layout/style/nsRuleNode.cpp#l3266

Comment 11

[Mats Palmgren (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7145e633829

Comment 12

[Mats Palmgren (inactive)]

Attachment: fix, v2

Dropped the offending MOZ_ASSERTs from the first patch and
s/NSCoordSaturatingNonnegativeMultiply/NSCoordSaturatingMultiply/
and added some comments explaining the situation.

Try run looks green so far.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b328d0b448c

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4b328d0b448c