| Summary: | Nullopt in DOMSelection::getRangeAt | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||||||||
| Component: | HTML Editing | Assignee: | Frédéric Wang (:fredw) <fred.wang> | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | achristensen, bfulgham, cgarcia, ews-feeder, fred.wang, gpoo, product-security, rbuis, svillar, webkit-bug-importer, wenson_hsieh | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=221719 | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
Ryosuke Niwa
2021-03-17 00:54:29 PDT
I can reproduce with WebKitTestRunner and DumpRenderTree at r274459. This could be a duplicate of the bug 221719 given the root cause analysis there. Created attachment 423597 [details] Reduced testcase Here is a reduced version of the test. > This could be a duplicate of the bug 221719 given the root cause analysis there. The current version of the patch there does not help, but let's analyze this more. It seems the issue is that we are extending the selection toward a pseudo element. Testcase is hitting the following assert in debug:
ASSERTION FAILED: !m_anchorNode || !m_anchorNode->isPseudoElement()
#0 0x00007f012e2329f6 in WebCore::Position::Position(WebCore::Node*, WebCore::Position::AnchorType)
(this=0x7ffea5d8e9e0, anchorNode=0x7f01209d8100, anchorType=WebCore::Position::PositionIsAfterAnchor) at ../../Source/WebCore/dom/Position.cpp:123
#1 0x00007f012daec30f in WebCore::positionAfterNode(WebCore::Node*)
(anchorNode=0x7f01209d8100) at ../../Source/WebCore/dom/Position.h:315
#2 0x00007f012e3c6ccc in WebCore::endPositionForLine(WebCore::VisiblePosition const&, WebCore::LineEndpointComputationMode)
(c=..., mode=WebCore::UseLogicalOrdering)
at ../../Source/WebCore/editing/VisibleUnits.cpp:852
#3 0x00007f012e3c6e9f in WebCore::endOfLine(WebCore::VisiblePosition const&, WebCore::LineEndpointComputationMode, bool*)
(c=..., mode=WebCore::UseLogicalOrdering, reachedBoundary=0x0)
at ../../Source/WebCore/editing/VisibleUnits.cpp:868
#4 0x00007f012e3c71e6 in WebCore::logicalEndOfLine(WebCore::VisiblePosition const&, bool*) (currentPosition=..., reachedBoundary=0x0)
at ../../Source/WebCore/editing/VisibleUnits.cpp:914
#5 0x00007f012e341865 in WebCore::FrameSelection::modifyExtendingForward(WebCore::TextGranularity)
Created attachment 423723 [details] Patch This fixes the release crash of attachment 423446 [details] and debug assertion of 423597 ; but there is a separate debug assert reached with the original test. Created attachment 423725 [details]
Reduced testcase for SHOULD NEVER BE REACHED
#0 WTFCrash() () at ../../Source/WTF/wtf/Assertions.cpp:295
#1 0x00007fa184a0c6e1 in CRASH_WITH_INFO(...) ()
at DerivedSources/ForwardingHeaders/wtf/Assertions.h:713
#2 0x00007fa188af0b6b in WebCore::RenderElement::visibleInViewportStateChanged() (this=0x7fa17a1fc610)
at ../../Source/WebCore/rendering/RenderElement.cpp:1394
#3 0x00007fa188af0b33 in WebCore::RenderElement::setVisibleInViewportState(WebCore::VisibleInViewportState)
(this=0x7fa17a1fc610, state=WebCore::VisibleInViewportState::No)
at ../../Source/WebCore/rendering/RenderElement.cpp:1389
#4 0x00007fa188cb74dd in WebCore::RenderView::updateVisibleViewportRect(WebCore::IntRect const&) (this=0x7fa17a219270, visibleRect=...)
at ../../Source/WebCore/rendering/RenderView.cpp:758
#5 0x00007fa1883dfea0 in operator()(WebCore::FrameView&, WebCore::IntRect const&) const (__closure=0x7fa1132a13d8, frameView=..., visibleRect=...)
at ../../Source/WebCore/page/FrameView.cpp:2029
#6 0x00007fa1883f3ba4 in WTF::Detail::CallableWrapper<WebCore::FrameView::viewportContentsChanged()::<lambda(WebCore::FrameView&, const WebCore::IntRect&)>, void, WebCore::FrameView&, const WebCore::IntRect&>::call(WebCore::FrameView &, const WebCore::IntRect &) (this=0x7fa1132a13d0, in#0=..., in#1=...)
at DerivedSources/ForwardingHeaders/wtf/Function.h:52
#7 0x00007fa1883fb41b in WTF::Function<void (WebCore::FrameView&, WebCore::IntRect const&)>::operator()(WebCore::FrameView&, WebCore::IntRect const&) const
(this=0x7fff30554748, in#0=..., in#1=...)
at DerivedSources/ForwardingHeaders/wtf/Function.h:83
#8 0x00007fa1883e26ab in WebCore::FrameView::applyRecursivelyWithVisibleRect(WTF::Function<void (WebCore::FrameView&, WebCore::IntRect const&)> const&)
(this=0x7fa17a218010, apply=...)
at ../../Source/WebCore/page/FrameView.cpp:2539
#9 0x00007fa1883dff2e in WebCore::FrameView::viewportContentsChanged()
(this=0x7fa17a218010) at ../../Source/WebCore/page/FrameView.cpp:2024
#10 0x00007fa1883e50ee in WebCore::FrameView::performPostLayoutTasks()
(this=0x7fa17a218010) at ../../Source/WebCore/page/FrameView.cpp:3363
#11 0x00007fa1883f0304 in WebCore::FrameViewLayoutContext::runAsynchronousTasks() (this=0x7fa17a218158)
at ../../Source/WebCore/page/FrameViewLayoutContext.cpp:301
#12 0x00007fa1883f0251 in WebCore::FrameViewLayoutContext::runOrScheduleAsynchronousTasks() (this=0x7fa17a218158)
at ../../Source/WebCore/page/FrameViewLayoutContext.cpp:287
#13 0x00007fa1883f009d in WebCore::FrameViewLayoutContext::layout()
(this=0x7fa17a218158)
at ../../Source/WebCore/page/FrameViewLayoutContext.cpp:260
#14 0x00007fa1883f0dc1 in WebCore::FrameViewLayoutContext::layoutTimerFired()
(this=0x7fa17a218158)
at ../../Source/WebCore/page/FrameViewLayoutContext.cpp:468
Comment on attachment 423723 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423723&action=review > Source/WebCore/editing/VisibleUnits.cpp:751 > + if (!startNode) This isn't right. We should be skipping pseudo elements and getting to the real element instead. See the loop right below this code. Created attachment 423865 [details]
Patch
Comment on attachment 423723 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423723&action=review >> Source/WebCore/editing/VisibleUnits.cpp:751 >> + if (!startNode) > > This isn't right. We should be skipping pseudo elements and getting to the real element instead. See the loop right below this code. Done. Comment on attachment 423865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=423865&action=review > Source/WebCore/ChangeLog:10 > + release mode. This patches fixes start/endPositionForLine to avoid that issue. Nit: This patch? Created attachment 424000 [details]
Patch for landing
Committed r274863: <https://commits.webkit.org/r274863> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424000 [details]. |