"ABORT: negative scaling factors must be handled manually" take two

RESOLVED WORKSFORME

Status

()

--
critical
RESOLVED WORKSFORME
9 years ago
3 years ago

People

(Reporter: jruderman, Assigned: Waldo)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
assertion, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

9 years ago
Created attachment 400577 [details]
testcase (causes abort in debug firefox)

###!!! ABORT: negative scaling factors must be handled manually: 'aScale >= 0.0f', file ../../dist/include/nsCoord.h, line 127

(Bug 508325 fixed this abort for another testcase.)
(Assignee)

Comment 1

9 years ago
Created attachment 400650 [details] [diff] [review]
Patch

There's a non-saturating scale with a bad value of the browser.display.base_font_scaler preference, but since from what I can tell the preference only existed for NS4 backwards-compat (and under a different name completely!) I don't care.  That'll die with bug 516547, and it's not important enough for anyone with better things to do to fix it.
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Attachment #400650 - Flags: review?
(Assignee)

Updated

9 years ago
Attachment #400650 - Flags: review? → review?(bzbarsky)
(Assignee)

Comment 2

9 years ago
Comment on attachment 400650 [details] [diff] [review]
Patch

>diff --git a/layout/style/nsRuleNode.cpp b/layout/style/nsRuleNode.cpp

>@@ -2573,7 +2573,7 @@ nsRuleNode::SetFontSize(nsPresContext* a
>       if (NS_STYLE_FONT_SIZE_LARGER == value) {
>         *aSize = nsStyleUtil::FindNextLargerFontSize(parentSize,
>                          baseSize, scaleFactor, aPresContext, eFontSize_CSS);
>-        NS_ASSERTION(*aSize > parentSize,
>+        NS_ASSERTION(*aSize >= parentSize,
>                      "FindNextLargerFontSize failed");

Scaling up nscoord_MAX doesn't produce a scaled size that's bigger, hit by font-size-monster-2.html.

>@@ -2602,8 +2602,9 @@ nsRuleNode::SetFontSize(nsPresContext* a
>     // Note that % units use the parent's size unadjusted for scriptlevel
>     // changes. A scriptlevel change between us and the parent is simply
>     // ignored.
>-    *aSize = NSToCoordRound(aParentSize *
>-                            aFontData.mSize.GetPercentValue());
>+    *aSize =
>+        NSCoordSaturatingNonnegativeMultiply(aParentSize,
>+                                             aFontData.mSize.GetPercentValue());

This is required by font-size-monster-1.html, that is, the original testcase here.


>diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp

> nsStyleFont::ZoomText(nsPresContext *aPresContext, nscoord aSize)
> {
>-  return nscoord(float(aSize) * aPresContext->TextZoom());
>+  return NSCoordSaturatingNonnegativeMultiply(aSize, aPresContext->TextZoom());
> }

This is required by font-size-monster-3.html.


> nsStyleFont::UnZoomText(nsPresContext *aPresContext, nscoord aSize)
> {
>-  return nscoord(float(aSize) / aPresContext->TextZoom());
>+  return NSCoordSaturatingDivide(aSize, aPresContext->TextZoom());
> }

This is required by font-size-monster-4.html.


>diff --git a/layout/style/nsStyleUtil.cpp b/layout/style/nsStyleUtil.cpp
>--- a/layout/style/nsStyleUtil.cpp
>+++ b/layout/style/nsStyleUtil.cpp
>@@ -366,7 +366,7 @@ nscoord nsStyleUtil::FindNextLargerFontS
>       largerSize = indexFontSize + NSToCoordRound(relativePosition * (largerIndexFontSize - indexFontSize));      
>     }
>     else {  // larger than HTML table, increase by 50%
>-      largerSize = NSToCoordRound(float(aFontSize) * 1.5);
>+      largerSize = NSCoordSaturatingNonnegativeMultiply(aFontSize, 1.5);
>     }
>   }
>   else { // smaller than HTML table, increase by 1px

This is required by font-size-monster-2.html.
(Assignee)

Comment 3

9 years ago
Created attachment 400652 [details] [diff] [review]
Missed a file last time
Attachment #400650 - Attachment is obsolete: true
Attachment #400652 - Flags: review?(bzbarsky)
Attachment #400650 - Flags: review?(bzbarsky)
(Assignee)

Updated

9 years ago
Component: Graphics → Layout: Text
OS: Mac OS X → All
QA Contact: thebes → layout.fonts-and-text
Hardware: x86 → All
Sorry for the lag here but....  NSCoordSaturatingDivide no longer seems to exist.  And I'm confused by the code there and in _nscoordSaturatingMultiply (which does exist): why do we floor in the float case but round to nearest in the nscoord case?
Comment on attachment 400652 [details] [diff] [review]
Missed a file last time

r- based at least on the differential rounding behavior.
Attachment #400652 - Flags: review?(bzbarsky) → review-
(Reporter)

Comment 6

9 years ago
WFM
(Assignee)

Comment 7

9 years ago
I think I saw the changeset that fixed this go past in pushlog semi-recently-ish, with basically or identically the change here.  Need to check up on that and get the tests in in any case...
No longer happens on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Flags: in-testsuite+
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.