Closed Bug 1464113 Opened 6 years ago Closed 6 years ago

ImageShapeInfo with shape-margin can't handle shapes offset left or above their margin rects

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Version:
61 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla62
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox60 --- unaffected
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: tsmith, Assigned: bradwerth)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(3 files)

testcase.html
176 bytes, text/html
Details
Bug 1464113 Part 1: Make ImageShapeInfo tolerate shapes that are shifted left or above their margin rects.
59 bytes, text/x-review-board-request
dholbert
: review+
Details
Bug 1464113 Part 2: Add a WPT reftest of a shape-outside: image with a negative left offset relative to its containing block.
59 bytes, text/x-review-board-request
dholbert
: review+
Details
Attached file testcase.html — 
Reproduced with m-c:
BuildID=20180523220103
SourceStamp=47e81ea1ef10189ef210867934bf36e14cf223dc

Assertion failure: offsetPoint.x >= 0 && offsetPoint.y >= 0 (aContentRect should be within aMarginRect, which we need for our math to make sense.), at src/layout/generic/nsFloatManager.cpp:1935

#0 nsFloatManager::ImageShapeInfo::ImageShapeInfo(unsigned char*, int, mozilla::gfx::IntSizeTyped<mozilla::LayoutDevicePixel> const&, int, float, int, nsRect const&, nsRect const&, mozilla::WritingMode, nsSize const&) src/layout/generic/nsFloatManager.cpp:1843:1
#1 mozilla::detail::UniqueSelector<nsFloatManager::ImageShapeInfo>::SingleObject mozilla::MakeUnique<nsFloatManager::ImageShapeInfo, unsigned char*&, int&, mozilla::gfx::IntSizeTyped<mozilla::LayoutDevicePixel>&, int&, float&, int&, nsRect&, nsRect&, mozilla::WritingMode&, nsSize const&>(unsigned char*&, int&, mozilla::gfx::IntSizeTyped<mozilla::LayoutDevicePixel>&, int&, float&, int&, nsRect&, nsRect&, mozilla::WritingMode&, nsSize const&) src/obj-firefox/dist/include/mozilla/UniquePtr.h:680:27
#2 nsFloatManager::ShapeInfo::CreateImageShape(mozilla::UniquePtr<nsStyleImage, mozilla::DefaultDelete<nsStyleImage> > const&, float, int, nsIFrame*, mozilla::LogicalRect const&, mozilla::WritingMode, nsSize const&) src/layout/generic/nsFloatManager.cpp:2845:10
#3 nsFloatManager::FloatInfo::FloatInfo(nsIFrame*, int, int, mozilla::LogicalRect const&, mozilla::WritingMode, nsSize const&) src/layout/generic/nsFloatManager.cpp:2374:20
#4 nsFloatManager::AddFloat(nsIFrame*, mozilla::LogicalRect const&, mozilla::WritingMode, nsSize const&) src/layout/generic/nsFloatManager.cpp:260:13
#5 mozilla::BlockReflowInput::FlowAndPlaceFloat(nsIFrame*) src/layout/generic/BlockReflowInput.cpp:994:19
#6 mozilla::BlockReflowInput::AddFloat(nsLineLayout*, nsIFrame*, int) src/layout/generic/BlockReflowInput.cpp:627:14
#7 nsLineLayout::ReflowFrame(nsIFrame*, nsReflowStatus&, mozilla::ReflowOutput*, bool&) src/layout/generic/nsLineLayout.cpp:966:11
#8 nsBlockFrame::ReflowInlineFrame(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsIFrame*, LineReflowStatus*) src/layout/generic/nsBlockFrame.cpp:4158:15
#9 nsBlockFrame::DoReflowInlineFrames(mozilla::BlockReflowInput&, nsLineLayout&, nsLineList_iterator, nsFlowAreaRect&, int&, nsFloatManager::SavedState*, bool*, LineReflowStatus*, bool) src/layout/generic/nsBlockFrame.cpp:3958:5
#10 nsBlockFrame::ReflowInlineFrames(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3832:9
#11 nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2816:5
#12 nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2352:7
#13 nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1225:3
#14 nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, mozilla::ReflowInput&, nsReflowStatus&, mozilla::BlockReflowInput&) src/layout/generic/nsBlockReflowContext.cpp:306:11
#15 nsBlockFrame::ReflowBlockFrame(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:3463:11
#16 nsBlockFrame::ReflowLine(mozilla::BlockReflowInput&, nsLineList_iterator, bool*) src/layout/generic/nsBlockFrame.cpp:2813:5
#17 nsBlockFrame::ReflowDirtyLines(mozilla::BlockReflowInput&) src/layout/generic/nsBlockFrame.cpp:2352:7
#18 nsBlockFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsBlockFrame.cpp:1225:3
#19 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14
#20 nsCanvasFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsCanvasFrame.cpp:713:5
#21 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, mozilla::WritingMode const&, mozilla::LogicalPoint const&, nsSize const&, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:951:14
#22 nsHTMLScrollFrame::ReflowScrolledFrame(mozilla::ScrollReflowInput*, bool, bool, mozilla::ReflowOutput*, bool) src/layout/generic/nsGfxScrollFrame.cpp:555:3
#23 nsHTMLScrollFrame::ReflowContents(mozilla::ScrollReflowInput*, mozilla::ReflowOutput const&) src/layout/generic/nsGfxScrollFrame.cpp:678:3
#24 nsHTMLScrollFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/nsGfxScrollFrame.cpp:1055:3
#25 nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, int, int, unsigned int, nsReflowStatus&, nsOverflowContinuationTracker*) src/layout/generic/nsContainerFrame.cpp:995:14
#26 mozilla::ViewportFrame::Reflow(nsPresContext*, mozilla::ReflowOutput&, mozilla::ReflowInput const&, nsReflowStatus&) src/layout/generic/ViewportFrame.cpp:335:7
#27 mozilla::PresShell::DoReflow(nsIFrame*, bool) src/layout/base/PresShell.cpp:8942:11
#28 mozilla::PresShell::ProcessReflowCommands(bool) src/layout/base/PresShell.cpp:9115:24
#29 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4332:11
#30 nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1923:16
#31 mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:301:7
#32 mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:320:5
#33 mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:760:5
#34 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:673:35
#35 mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:574:9
#36 mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) src/layout/ipc/VsyncChild.cpp:68:16
#37 mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:167:20
#38 mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:1988:28
#39 mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2136:25
#40 mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2066:17
#41 mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:1912:5
#42 mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:1945:15
#43 nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1090:14
#44 NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
#45 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
#46 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:326:10
#47 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299:3
#48 nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:157:27
#49 XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:893:22
#50 mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:269:9
#51 MessageLoop::RunInternal() src/ipc/chromium/src/base/message_loop.cc:326:10
#52 MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299:3
#53 XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:719:34
#54 content_process_main(mozilla::Bootstrap*, int, char**) src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
#55 main src/browser/app/nsBrowserApp.cpp:282:18
#56 __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
#57 _start (firefox+0x423434)
Flags: in-testsuite?
Brad, looks like this one is shape-outside-related.
Flags: needinfo?(bwerth)
Assignee: nobody → bwerth
Flags: needinfo?(bwerth)
Looks like in the process of preventing the ImageShapeInfo distance field code from addressing locations outside the distance field, I made it too cautious about handling shapes that are shifted left or above their margin rects. The assert can go away and some of the other math has to be casted differently. I'll also add tests.
Summary: Assertion failure: offsetPoint.x >= 0 && offsetPoint.y >= 0 (aContentRect should be within aMarginRect, which we need for our math to make sense.), at src/layout/generic/nsFloatManager.cpp:1935 → ImageShapeInfo with shape-margin can't handle shapes offset left or above their margin rects
Attachment #8980811 - Flags: review?(dholbert)
Attachment #8980812 - Flags: review?(dholbert)
Comment on attachment 8980811 [details]
Bug 1464113 Part 1: Make ImageShapeInfo tolerate shapes that are shifted left or above their margin rects.

https://reviewboard.mozilla.org/r/246984/#review254174
Attachment #8980811 - Flags: review?(dholbert) → review+
Comment on attachment 8980812 [details]
Bug 1464113 Part 2: Add a WPT reftest of a shape-outside: image with a negative left offset relative to its containing block.

https://reviewboard.mozilla.org/r/246986/#review254248
Attachment #8980812 - Flags: review?(dholbert) → review+
Pushed by bwerth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5c8b1c2a4200
Part 1: Make ImageShapeInfo tolerate shapes that are shifted left or above their margin rects. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/494c6daf83df
Part 2: Add a WPT reftest of a shape-outside: image with a negative left offset relative to its containing block. r=dholbert
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/11262 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
https://hg.mozilla.org/mozilla-central/rev/5c8b1c2a4200
https://hg.mozilla.org/mozilla-central/rev/494c6daf83df
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Does this need an uplift request or can it ride the trains?
Blocks: 1457288
status-firefox60: --- → unaffected
status-firefox61: --- → affected
status-firefox-esr52: --- → unaffected
status-firefox-esr60: --- → unaffected
Flags: needinfo?(bwerth)
Flags: in-testsuite?
Flags: in-testsuite+
Version: unspecified → 61 Branch
Since it's just an assertion, and the feature is only enabled by default in Nightly 61, this can ride the trains to 62.
Flags: needinfo?(bwerth)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: