If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

"ABORT: negative scaling factors must be handled manually" with background, -moz-column

NEW
Assigned to

Status

()

Core
Layout
--
critical
6 years ago
21 days ago

People

(Reporter: Jesse Ruderman, Assigned: mats)

Tracking

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

Trunk
x86_64
Mac OS X
assertion, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Created attachment 580582 [details]
testcase (asserts fatally when loaded)

###!!! ABORT: negative scaling factors must be handled manually: 'aScale >= 0.0f', file nsCoord.h, line 138
(Reporter)

Comment 1

6 years ago
Created attachment 580583 [details]
stack trace
(Assignee)

Comment 2

5 years ago
Created attachment 734275 [details] [diff] [review]
fix+test

https://tbpl.mozilla.org/?tree=Try&rev=8da8879637d8
Assignee: nobody → matspal
Attachment #734275 - Flags: review?(roc)
Comment on attachment 734275 [details] [diff] [review]
fix+test

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

Wouldn't it make more sense to fix this in nsCSSRendering::ComputeBackgroundPositioningArea?
(Assignee)

Comment 4

5 years ago
Created attachment 734940 [details] [diff] [review]
fix+test, v2

OK.  The only other caller of ComputeBackgroundPositioningArea
is nsDisplayBackgroundImage::GetPositioningArea() and I figured the
mIsThemed path should be consistent with the non-mIsThemed path so
I clamped that too.

https://tbpl.mozilla.org/?tree=Try&rev=e813a1025a29
Attachment #734275 - Attachment is obsolete: true
Attachment #734275 - Flags: review?(roc)
Attachment #734940 - Flags: review?(roc)
Comment on attachment 734940 [details] [diff] [review]
fix+test, v2

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

::: layout/base/nsCSSRendering.cpp
@@ +4601,5 @@
>  nsSize
>  nsImageRenderer::ComputeSize(const nsStyleBackground::Size& aLayerSize,
>                               const nsSize& aBgPositioningArea)
>  {
> +  MOZ_ASSERT(aBgPositioningArea.width >= 0 && aBgPositioningArea.height >= 0);

I think this should be non-fatal. It doesn't matter much if it's negative here I think.

::: layout/base/nsDisplayList.cpp
@@ +2023,5 @@
>  {
>    if (mIsThemed) {
> +    nsRect bgRect(ToReferenceFrame(), mFrame->GetSize());
> +    bgRect.width = std::max(0, bgRect.width);
> +    bgRect.height = std::max(0, bgRect.height);

Why is this necessary? Frame sizes should never be negative. Do we have known situations where frame sizes go negative?
Attachment #734940 - Flags: review?(roc)
See Also: → bug 1395974
You need to log in before you can comment on or make changes to this bug.