Closed Bug 843072 Opened 7 years ago Closed 7 years ago

Crash with getExtentOfChar

Categories

(Core :: SVG, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: jruderman, Assigned: longsonr)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase)

Crash Data

Attachments

(4 files, 1 obsolete file)

With:
  user_pref("svg.text.css-frames.enabled", true);

This one seems to be a regression from *after* svgtext:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e8f8a3f6f1f6&tochange=0bb9e8c676da

Maybe it's a regression from bug 842119?
Attached file stack (gdb)
Crash Signature: [@ nsTextFrame::EnsureTextRun]
On Windows: bp-10ecd62e-bfda-4c75-bf67-16ce82130220
Crash Signature: [@ nsTextFrame::EnsureTextRun] → [@ nsTextFrame::EnsureTextRun] [@ nsTextFrame::EnsureTextRun(nsTextFrame::TextRunType, gfxContext*, nsIFrame*, nsLineList_iterator const*, unsigned int*) ]
OS: Mac OS X → All
Hardware: x86_64 → All
Attached patch patch (obsolete) — Splinter Review
Because there's no text at all and we are asking about character 0 (which clearly doesn't exist), we don't need to advance (Next(0) returns true), but even so we're still beyond the end of the text.
Assignee: nobody → longsonr
Attachment #720142 - Flags: review?(dholbert)
Comment on attachment 720142 [details] [diff] [review]
patch


All looks trivially-good (from the explanation in comment 4) except for this first part that I'm not 100% sure about:

>diff --git a/layout/svg/nsSVGTextFrame2.cpp b/layout/svg/nsSVGTextFrame2.cpp
>--- a/layout/svg/nsSVGTextFrame2.cpp
>+++ b/layout/svg/nsSVGTextFrame2.cpp
>@@ -3650,17 +3650,17 @@ nsSVGTextFrame2::GetSubStringLength(nsIC
>     return 0.0f;
>   }
> 
>   // Convert charnum/nchars from addressable characters relative to
>   // aContent to global character indices.
>   CharIterator chit(this, CharIterator::eAddressable, aContent);
>   if (!chit.AdvanceToSubtree() ||
>       !chit.Next(charnum) ||
>-      chit.IsAfterSubtree()) {
>+      chit.AtEnd()) {
>     return 0.0f;
>   }

The old check, IsAfterSubtree(), is a different (more specific) check than AtEnd() -- so this change is relaxing that check here. Is that intended?  (Maybe we should be checking both AtEnd && IsAfterSubtree?)
That's a typo. I meant to add the extra condition, not replace.
Attached patch updated patchSplinter Review
Attachment #720142 - Attachment is obsolete: true
Attachment #720142 - Flags: review?(dholbert)
Attachment #720239 - Flags: review?(dholbert)
Comment on attachment 720239 [details] [diff] [review]
updated patch

Cool -- r=me on the updated patch then.
Attachment #720239 - Flags: review?(dholbert) → review+
https://hg.mozilla.org/mozilla-central/rev/7d93ba84ce2f
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.