Crash with getExtentOfChar

RESOLVED FIXED in mozilla22

Status

()

Core
SVG
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: longsonr)

Tracking

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

Trunk
mozilla22
crash, regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

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

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?
(Reporter)

Comment 1

5 years ago
Created attachment 716052 [details]
stack (gdb)
(Reporter)

Comment 2

5 years ago
Created attachment 716053 [details]
stack (context + blame)
(Reporter)

Updated

5 years ago
Crash Signature: [@ nsTextFrame::EnsureTextRun]

Comment 3

5 years ago
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
(Assignee)

Comment 4

5 years ago
Created attachment 720142 [details] [diff] [review]
patch

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?)
(Assignee)

Comment 6

5 years ago
That's a typo. I meant to add the extra condition, not replace.
(Assignee)

Comment 7

5 years ago
Created attachment 720239 [details] [diff] [review]
updated patch
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+
(Assignee)

Comment 9

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=8eaa8c3c12bb
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/7d93ba84ce2f
https://hg.mozilla.org/mozilla-central/rev/7d93ba84ce2f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
You need to log in before you can comment on or make changes to this bug.