Closed Bug 828054 Opened 7 years ago Closed 7 years ago

Assertion failure: "Strong directional characters before aStartAfterNode" with nested dir=auto

Categories

(Core :: DOM: Core & HTML, defect, critical)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla21
Tracking Status
firefox18 --- unaffected
firefox19 --- unaffected
firefox20 --- fixed
firefox21 --- verified
firefox-esr17 --- unaffected

People

(Reporter: jruderman, Assigned: smontagu)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase)

Attachments

(3 files, 1 obsolete file)

Attached file testcase
Assertion failure: GetDirectionFromText(child->GetText()) == eDir_NotSet (Strong directional characters before aStartAfterNode), at content/base/src/DirectionalityUtils.cpp:393

(Similar to bug 822723, which smontagu fixed a few weeks ago.)
Attached patch Patch (obsolete) — Splinter Review
On reflection I believe that the whole optimization that this assertion is part of is not sustainable: I think finding the correct node "after" aStartNode to start walking the descendant tree from is too complicated, and getting it right will be less performant than walking the whole descendant tree.
Assignee: nobody → smontagu
Attachment #699828 - Flags: review?(ehsan)
Wrong patch?
Attached file Another testcase
For example, in this variation on Jesse's testcase, the current code resets the text node setting the direction of |outer| from the text node with "ا" to the one with "پ"، which is Just Wrong.
Comment on attachment 699828 [details] [diff] [review]
Patch

empty patch.
Attachment #699828 - Flags: review?(ehsan) → review-
Attached patch PatchSplinter Review
Sorry, didn't hg qref
Attachment #699828 - Attachment is obsolete: true
Attachment #700463 - Flags: review?(ehsan)
Comment on attachment 700463 [details] [diff] [review]
Patch

Review of attachment 700463 [details] [diff] [review]:
-----------------------------------------------------------------

Fair enough...
Attachment #700463 - Flags: review?(ehsan) → review+
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/cdfcfefca432
https://hg.mozilla.org/mozilla-central/rev/62060132c75b
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment on attachment 700463 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 548206
User impact if declined: Elements with dir=auto could get the wrong direction in some dynamic situations
Testing completed (on m-c, etc.): Baked on m-c since 2013-01-13
Risk to taking this patch (and alternatives if risky): None known.
String or UUID changes made by this patch: None
Attachment #700463 - Flags: approval-mozilla-aurora?
Attachment #700463 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Issue reproducible with the debug build from 2013-01-08. (assertion failure + Firefox crash)

Verified fixed with Firefox 21 latest debug build, on Mac OSX 10.8.3 (no assertion failure + no Firefox crash with the testcase provided)

Build ID: 20130426142004
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.