WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
130844
ASSERTION FAILED: candidate.isCandidate() in WebCore::canonicalizeCandidate
https://bugs.webkit.org/show_bug.cgi?id=130844
Summary
ASSERTION FAILED: candidate.isCandidate() in WebCore::canonicalizeCandidate
Renata Hodovan
Reported
2014-03-27 10:00:42 PDT
Created
attachment 227952
[details]
Test case The test hits the assertion: <body contenteditable="true"> <fieldset> <sup contenteditable="false"> <svg> <polyline onload='document.execCommand("selectall", true, null)'> The related backtrace: ASSERTION FAILED: candidate.isCandidate() /home/reni2/data/REPOS/webkit_sec/Source/WebCore/editing/VisiblePosition.cpp(496) : WebCore::Position WebCore::canonicalizeCandidate(const WebCore::Position&) 1 0x7ffff5ed9db5 WTFCrash 2 0x7ffff10e6977 3 0x7ffff10e6b8a WebCore::VisiblePosition::canonicalPosition(WebCore::Position const&) 4 0x7ffff10e4a8d WebCore::VisiblePosition::init(WebCore::Position const&, WebCore::EAffinity) 5 0x7ffff10e4a53 WebCore::VisiblePosition::VisiblePosition(WebCore::Position const&, WebCore::EAffinity) 6 0x7ffff10e8adc WebCore::VisibleSelection::setBaseAndExtentToDeepEquivalents() 7 0x7ffff10e9b57 WebCore::VisibleSelection::validate(WebCore::TextGranularity) 8 0x7ffff10e7cac WebCore::VisibleSelection::VisibleSelection(WebCore::Position const&, WebCore::Position const&, WebCore::EAffinity, bool) 9 0x7ffff10e7f15 WebCore::VisibleSelection::selectionFromContentsOfNode(WebCore::Node*) 10 0x7ffff10a9f5c WebCore::FrameSelection::selectAll() 11 0x7ffff109d15e 12 0x7ffff109eb24 WebCore::Editor::Command::execute(WTF::String const&, WebCore::Event*) const 13 0x7ffff0f58f4c WebCore::Document::execCommand(WTF::String const&, bool, WTF::String const&) 14 0x7ffff1f340fb WebCore::jsDocumentPrototypeFunctionExecCommand(JSC::ExecState*) 15 0x7fff9b6ce0b4 Program received signal SIGSEGV, Segmentation fault. 0x00007ffff5ed9dba in WTFCrash () at /home/reni2/data/REPOS/webkit_sec/Source/WTF/wtf/Assertions.cpp:333 333 *(int *)(uintptr_t)0xbbadbeef = 0; (gdb) ERR<26821>:eet eet_lib.c:668 eet_shutdown() File '/home/reni2/.cache/efreet/icon_themes_reni2.eet' is still open ! bt #0 0x00007ffff5ed9dba in WTFCrash () at /home/reni2/data/REPOS/webkit_sec/Source/WTF/wtf/Assertions.cpp:333 #1 0x00007ffff10e6977 in WebCore::canonicalizeCandidate (candidate=...) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/editing/VisiblePosition.cpp:496 #2 0x00007ffff10e6b8a in WebCore::VisiblePosition::canonicalPosition (this=0x7fffffffb290, passedPosition=...) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/editing/VisiblePosition.cpp:533 #3 0x00007ffff10e4a8d in WebCore::VisiblePosition::init (this=0x7fffffffb290, position=..., affinity=WebCore::DOWNSTREAM) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/editing/VisiblePosition.cpp:58 #4 0x00007ffff10e4a53 in WebCore::VisiblePosition::VisiblePosition (this=0x7fffffffb290, pos=..., affinity=WebCore::DOWNSTREAM) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/editing/VisiblePosition.cpp:51 #5 0x00007ffff10e8adc in WebCore::VisibleSelection::setBaseAndExtentToDeepEquivalents (this=0x7fffffffb3b0) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/editing/VisibleSelection.cpp:257 #6 0x00007ffff10e9b57 in WebCore::VisibleSelection::validate (this=0x7fffffffb3b0, granularity=WebCore::CharacterGranularity) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/editing/VisibleSelection.cpp:427 #7 0x00007ffff10e7cac in WebCore::VisibleSelection::VisibleSelection (this=0x7fffffffb3b0, base=..., extent=..., affinity=WebCore::DOWNSTREAM, isDirectional=false) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/editing/VisibleSelection.cpp:66 #8 0x00007ffff10e7f15 in WebCore::VisibleSelection::selectionFromContentsOfNode (node=0x9b8ec0) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/editing/VisibleSelection.cpp:99 #9 0x00007ffff10a9f5c in WebCore::FrameSelection::selectAll (this=0x8a0b40) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/editing/FrameSelection.cpp:1690 #10 0x00007ffff109d15e in WebCore::executeSelectAll (frame=...) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/editing/EditorCommand.cpp:1021 #11 0x00007ffff109eb24 in WebCore::Editor::Command::execute (this=0x7fffffffb4b0, parameter=..., triggeringEvent= 0x0) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/editing/EditorCommand.cpp:1741 #12 0x00007ffff0f58f4c in WebCore::Document::execCommand (this=0x8cfd40, commandName=..., userInterface=true, value=...) at /home/reni2/data/REPOS/webkit_sec/Source/WebCore/dom/Document.cpp:4217 #13 0x00007ffff1f340fb in WebCore::jsDocumentPrototypeFunctionExecCommand (exec=0x7fffffffb5b0) at /home/reni2/data/REPOS/webkit_sec/WebKitBuild/Debug/DerivedSources/WebCore/JSDocument.cpp:4736 #14 0x00007fff9b6ce0b4 in ?? () #15 0x00007fffffffb610 in ?? () #16 0x00007ffff5ec4fb5 in llint_op_call () from /home/reni2/data/REPOS/webkit_sec/WebKitBuild/Debug/lib/libjavascriptcore_efl.so.0 #17 0x0000000000000000 in ?? ()
Attachments
Test case
(148 bytes, text/html)
2014-03-27 10:00 PDT
,
Renata Hodovan
no flags
Details
Test
(270 bytes, text/html)
2015-12-10 02:21 PST
,
Renata Hodovan
no flags
Details
Patch
(2.46 KB, patch)
2020-04-15 14:14 PDT
,
Jack
no flags
Details
Formatted Diff
Diff
Patch
(2.43 KB, patch)
2020-04-15 17:13 PDT
,
Jack
no flags
Details
Formatted Diff
Diff
Test result of insert-list-in-table-cell-07.html without patch.
(157.32 KB, image/png)
2020-04-15 19:26 PDT
,
Jack
no flags
Details
Test result of insert-list-in-table-cell-07.html with patch.
(146.34 KB, image/png)
2020-04-15 19:27 PDT
,
Jack
no flags
Details
Patch
(6.04 KB, patch)
2020-04-15 20:17 PDT
,
Jack
no flags
Details
Formatted Diff
Diff
Patch
(6.09 KB, patch)
2020-04-16 06:29 PDT
,
Jack
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Renata Hodovan
Comment 1
2015-12-10 02:21:46 PST
Created
attachment 267083
[details]
Test Replacing the old test case with a new one that still works.
Brent Fulgham
Comment 2
2016-08-03 14:07:54 PDT
This reproduces under
r204037
.
Radar WebKit Bug Importer
Comment 3
2016-08-03 14:10:39 PDT
<
rdar://problem/27685444
>
Jack
Comment 4
2020-04-15 13:44:58 PDT
The attached test case still reproduces on ToT.
Jack
Comment 5
2020-04-15 14:04:17 PDT
New radar for tracking this issue: <
rdar://59535009
>
Jack
Comment 6
2020-04-15 14:14:21 PDT
Created
attachment 396572
[details]
Patch
Jack
Comment 7
2020-04-15 14:16:20 PDT
Comment on
attachment 396572
[details]
Patch Temporary patch to check if any test fails.
Jack
Comment 8
2020-04-15 17:13:49 PDT
Created
attachment 396593
[details]
Patch
Jack
Comment 9
2020-04-15 18:51:04 PDT
The failed test "insert-list-in-table-cell-07.html" was added in
https://bugs.webkit.org/show_bug.cgi?id=174593
.
Geoffrey Garen
Comment 10
2020-04-15 19:10:40 PDT
I think the test difference may be harmless, or a slight improvement. Can you check whether the test looks visually the same if you open it in Safari? (The test content is the table at the top of the page.)
Jack
Comment 11
2020-04-15 19:26:57 PDT
Created
attachment 396605
[details]
Test result of insert-list-in-table-cell-07.html without patch.
Jack
Comment 12
2020-04-15 19:27:22 PDT
Created
attachment 396606
[details]
Test result of insert-list-in-table-cell-07.html with patch.
Jack
Comment 13
2020-04-15 19:29:20 PDT
The test results are visually the same.
Jack
Comment 14
2020-04-15 20:03:40 PDT
Root cause: Function PositionIterator::isCandidate() has different implementation comparing to Position::isCandidate(), when the candidate is a br element. The former is called to find candidate in function previousCandidate, and the assertion calls the later function and disqualify the br element as a candidate. Difference: 1. PositionIterator::isCandidate() if (renderer->isBR()) return !m_offsetInAnchor && !Position::nodeIsUserSelectNone(m_anchorNode->parentNode()); 2. Position::isCandidate() if (renderer->isBR()) return !m_offset && m_anchorType != PositionIsAfterAnchor && !nodeIsUserSelectNone(deprecatedNode()->parentNode());
Jack
Comment 15
2020-04-15 20:17:37 PDT
Created
attachment 396610
[details]
Patch
Geoffrey Garen
Comment 16
2020-04-15 21:54:53 PDT
Comment on
attachment 396610
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=396610&action=review
> Source/WebCore/dom/PositionIterator.cpp:152 > + Position candidate = *this; > + return candidate.isCandidate();
I think you can change this function into a one-liner: return Position(*this).isCandidate(); (You can remove the m_anchorNode check too.)
Jack
Comment 17
2020-04-16 06:24:55 PDT
Thanks Geoff. However, Position(*this) would crash if m_anchorNode is null. It the following change okay? return m_anchorNode ? Position(*this).isCandidate() : false; (In reply to Geoffrey Garen from
comment #16
)
> Comment on
attachment 396610
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=396610&action=review
> > > Source/WebCore/dom/PositionIterator.cpp:152 > > + Position candidate = *this; > > + return candidate.isCandidate(); > > I think you can change this function into a one-liner: > > return Position(*this).isCandidate(); > > (You can remove the m_anchorNode check too.)
Jack
Comment 18
2020-04-16 06:29:12 PDT
Created
attachment 396640
[details]
Patch
Geoffrey Garen
Comment 19
2020-04-16 10:26:37 PDT
> Thanks Geoff. However, Position(*this) would crash if m_anchorNode is null.
I see. I misread that. My instinct is that PositionIterator::operator Position() should just check for null and return Position(); but perhaps I've gotten too far afield. This patch seems fine as is.
Geoffrey Garen
Comment 20
2020-04-16 10:26:47 PDT
Comment on
attachment 396640
[details]
Patch r=me
Jack
Comment 21
2020-04-16 11:38:57 PDT
Thanks. Yeah, I assumed that too. And surprisingly fuzzer hasn't found it. Probably m_anchorNode is always checked before Position() is called? (In reply to Geoffrey Garen from
comment #19
)
> My instinct is that PositionIterator::operator Position() should just check > for null and return Position(); but perhaps I've gotten too far afield. This > patch seems fine as is.
EWS
Comment 22
2020-04-16 11:42:40 PDT
Committed
r260207
: <
https://trac.webkit.org/changeset/260207
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 396640
[details]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug