Crash with range, normalize

RESOLVED FIXED in mozilla20

Status

()

Core
DOM: Core & HTML
--
critical
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: Mats Palmgren (vacation - back in August))

Tracking

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

Trunk
mozilla20
assertion, crash, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 674394 [details]
testcase (crashes Firefox when loaded)

###!!! ASSERTION: ContentIterator stack underflow: '!aIndexes || !aIndexes->IsEmpty()', file content/base/src/nsContentIterator.cpp, line 715

Crash [@ nsContentIterator::NextNode]

This seems like the opposite of bug 803924.

Comment 1

5 years ago
On Windows: bp-9056da4d-fd84-47c8-a7f3-83fd92121024.
Crash Signature: [@ nsContentIterator::NextNode] → [@ nsContentIterator::NextNode] [@ nsContentIterator::NextNode(nsINode*, nsTArray<int, nsTArrayDefaultAllocator>*)]
OS: Mac OS X → All
Hardware: x86_64 → All
Assignee: nobody → matspal
Created attachment 679185 [details] [diff] [review]
fix + test

Green on Try:
https://tbpl.mozilla.org/?tree=Try&rev=3f92287de016
Attachment #679185 - Flags: review?(bugs)

Comment 3

5 years ago
Comment on attachment 679185 [details] [diff] [review]
fix + test

Could you explain this a bit.
I'm trying to figure out why mStartOffset > 0 and not >= 0.

Could you add some test where textnode is empty.
Attachment #679185 - Flags: review?(bugs) → review-
Created attachment 680543 [details] [diff] [review]
fix + test

(In reply to Olli Pettay [:smaug] from comment #3)
> Could you explain this a bit.
> I'm trying to figure out why mStartOffset > 0 and not >= 0.

Because a boundary point before the first child cannot possibly
be affected by a normalize(), so it's an optimization --
"removed == parentNode->GetChildAt(mStartOffset)" is always false
for mStartOffset == 0.

(I realized I could do the same in bug 803924, thanks!)

https://tbpl.mozilla.org/?tree=Try&rev=591739349e83
Attachment #679185 - Attachment is obsolete: true
Attachment #680543 - Flags: review?(bugs)
Aryeh, there's a similar bug in the spec for 'normalize()':
http://dom.spec.whatwg.org/#dom-node-normalize
After 7.2 you need to add something like:
* For each range whose start node is /context object/ and start offset
  equals the index of /current node/ in /context object/, set start node
  to /node/ and start offset to /length/
* ditto with s/start/end/
(In reply to Mats Palmgren [:mats] from comment #5)
> Aryeh, there's a similar bug in the spec for 'normalize()':
> http://dom.spec.whatwg.org/#dom-node-normalize
> After 7.2 you need to add something like:
> * For each range whose start node is /context object/ and start offset
>   equals the index of /current node/ in /context object/, set start node
>   to /node/ and start offset to /length/
> * ditto with s/start/end/

Ah, good point.  It's not quite right, because /current node/ doesn't have to be a child of /context object/, but it's true that we need to account for boundary points in the parent.  Sigh, ranges are way too complicated.  Filed a spec bug:

https://www.w3.org/Bugs/Public/show_bug.cgi?id=19946

If Anne doesn't handle it soon, I will when I have the time.

Comment 7

5 years ago
Anne has valid concerns in the W3 bug.

Comment 8

5 years ago
Comment on attachment 680543 [details] [diff] [review]
fix + test

uh, this is tricky. I hope I got this right.
Could you add a comment why > 0 is ok.
Attachment #680543 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #7)
> Anne has valid concerns in the W3 bug.

I don't understand them, as I said there.  Maybe you could explain why they're valid?
Yeah, ok, perhaps those concerns don't apply here after all. This normalize case is pretty unique.
(In reply to Olli Pettay [:smaug] from comment #8)
> Could you add a comment why > 0 is ok.

Added comment and some more tests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ee5de769c4a
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/6ee5de769c4a
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Blocks: 815065
Component: DOM: Traversal-Range → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.