Closed
Bug 1425959
Opened 6 years ago
Closed 6 years ago
Assertion failure: providerFrame && providerFrame->GetParent()-> IsFrameOfType(nsIFrame::eBlockFrame) (How could we get a ::first-line parent style without having a ::first-line provider frame?), at /src/layout/base/ServoRestyleManager.cpp:1618
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox57 | --- | wontfix |
firefox58 | --- | wontfix |
firefox59 | --- | fixed |
People
(Reporter: tsmith, Assigned: MatsPalmgren_bugz)
References
(Blocks 1 open bug)
Details
(Keywords: assertion, testcase)
Attachments
(2 files)
234 bytes,
text/html
|
Details | |
3.77 KB,
patch
|
emilio
:
review+
|
Details | Diff | Splinter Review |
Assertion failure: providerFrame && providerFrame->GetParent()-> IsFrameOfType(nsIFrame::eBlockFrame) (How could we get a ::first-line parent style without having a ::first-line provider frame?), at /src/layout/base/ServoRestyleManager.cpp:1618 #0 mozilla::ServoRestyleManager::DoReparentStyleContext(nsIFrame*, mozilla::ServoStyleSet&) /src/layout/base/ServoRestyleManager.cpp:1560:5 #1 mozilla::ServoRestyleManager::ReparentStyleContext(nsIFrame*) /src/layout/base/ServoRestyleManager.cpp:1521:3 #2 nsCSSFrameConstructor::CheckForFirstLineInsertion(nsIFrame*, nsFrameItems&) /src/layout/base/nsCSSFrameConstructor.cpp:11316:23 #3 nsCSSFrameConstructor::ContentRangeInserted(nsIContent*, nsIContent*, nsIContent*, nsILayoutHistoryState*, nsCSSFrameConstructor::InsertionKind, TreeMatchContext*) /src/layout/base/nsCSSFrameConstructor.cpp:8254:5 #4 mozilla::RestyleManager::ProcessRestyledFrames(nsStyleChangeList&) /src/layout/base/RestyleManager.cpp:1419:27 #5 mozilla::ServoRestyleManager::DoProcessPendingRestyles(mozilla::ServoTraversalFlags) /src/layout/base/ServoRestyleManager.cpp:1161:9 #6 mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) /src/layout/base/PresShell.cpp:4185:41 #7 nsRefreshDriver::Tick(long, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:1891:18 #8 nsRefreshDriver::Tick(long, mozilla::TimeStamp) /src/layout/base/nsRefreshDriver.cpp:1862:12 #9 nsRefreshDriver::FinishedWaitingForTransaction() /src/layout/base/nsRefreshDriver.cpp:2154:5 #10 mozilla::layers::ClientLayerManager::DidComposite(unsigned long, mozilla::TimeStamp const&, mozilla::TimeStamp const&) /src/gfx/layers/client/ClientLayerManager.cpp:520:32 #11 mozilla::layers::CompositorBridgeChild::RecvDidComposite(unsigned long const&, unsigned long const&, mozilla::TimeStamp const&, mozilla::TimeStamp const&) /src/gfx/layers/ipc/CompositorBridgeChild.cpp:542:8 #12 mozilla::layers::PCompositorBridgeChild::OnMessageReceived(IPC::Message const&) /src/obj-firefox/ipc/ipdl/PCompositorBridgeChild.cpp:1441:20 #13 mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) /src/ipc/glue/MessageChannel.cpp:2110:25 #14 mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) /src/ipc/glue/MessageChannel.cpp:2040:17 #15 mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) /src/ipc/glue/MessageChannel.cpp:1886:5 #16 mozilla::ipc::MessageChannel::MessageTask::Run() /src/ipc/glue/MessageChannel.cpp:1919:15 #17 nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1039:14 #18 NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:508:10 #19 mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21 #20 MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10 #21 MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3 #22 nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:157:27 #23 nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30 #24 XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4709:22 #25 XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4871:8 #26 XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4963:21 #27 do_main(int, char**, char**) /src/browser/app/nsBrowserApp.cpp:231:22 #28 main /src/browser/app/nsBrowserApp.cpp:304:16 #29 __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291 #30 _start (firefox+0x41f3d4)
Flags: in-testsuite?
Comment 1•6 years ago
|
||
Huh, so this is because the <button> style has the ALLOW_BLOCK_STYLES bit in the frame constructor, based on the assumption that it always create a block wrapper for its kids, yet we have code from bug 984869 which doesn't create a block frame, but a grid container frame...
Assignee | ||
Comment 2•6 years ago
|
||
Yeah, I think we need to enforce that the parent actually is a block frame too. I'll take a look...
Assignee: nobody → mats
Assignee | ||
Comment 3•6 years ago
|
||
Attachment #8937758 -
Flags: review?(emilio)
Comment 4•6 years ago
|
||
Comment on attachment 8937758 [details] [diff] [review] Only allow ::first-letter/line children when the parent frame is a block frame Review of attachment 8937758 [details] [diff] [review]: ----------------------------------------------------------------- Heh, I had pretty much pushed the same patch to try a bit ago :) ::: layout/base/crashtests/1425959.html @@ +8,5 @@ > +} > +</script> > +<body onload=go()> > +<button id="a"> > +<video id="b"> You can probably make this something that isn't a <video> so the test-case is a bit simpler, but no big deal if it takes much effort. ::: layout/base/nsCSSFrameConstructor.cpp @@ +10998,5 @@ > > // XXXbz ideally, this would do all the pushing of various > // containing blocks as needed, so callers don't have to do it... > > + const bool allowFirstPseudos = aAllowBlockStyles && Can you leave a comment saying that we need to check that the frame is actually a block because we may create non-block wrappers for <button>, or something along those lines?
Attachment #8937758 -
Flags: review?(emilio) → review+
Pushed by mpalmgren@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e28372a90098 Only allow ::first-letter/line children when the parent frame is a block frame. r=emilio
Assignee | ||
Comment 6•6 years ago
|
||
(In reply to Emilio Cobos Álvarez [:emilio] from comment #4) > You can probably make this something that isn't a <video> so the test-case > is a bit simpler, but no big deal if it takes much effort. Sure, the assertion also occurs with <span id="b">. > Can you leave a comment saying that we need to check that the frame is > actually a block because we may create non-block wrappers for <button>, or > something along those lines? OK, I added a comment and clarified that FCDATA_ALLOW_BLOCK_STYLES doesn't imply that the parent frame is an actual block (it probably used to, but I think it's less error prone if it doesn't now that we allow display:grid/flexbox in a bunch of more places).
Flags: in-testsuite? → in-testsuite+
Comment 7•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e28372a90098
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 8•6 years ago
|
||
mozregression says this first started asserting when bug 1388877 landed, but I'm guessing this isn't severe enough to warrant uplift consideration, especially this late in the cycle.
Blocks: 1388877
status-firefox57:
--- → wontfix
status-firefox58:
--- → wontfix
status-firefox-esr52:
--- → unaffected
Version: 59 Branch → 57 Branch
You need to log in
before you can comment on or make changes to this bug.
Description
•