Last Comment Bug 699353 - "ASSERTION: aOffset must be in the frame's range" with caret browsing
: "ASSERTION: aOffset must be in the frame's range" with caret browsing
Status: RESOLVED FIXED
: assertion, testcase
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- normal (vote)
: mozilla13
Assigned To: :Ehsan Akhgari (busy, don't ask for review please)
:
Mentors:
Depends on:
Blocks: 336383
  Show dependency treegraph
 
Reported: 2011-11-03 03:07 PDT by Jesse Ruderman
Modified: 2012-02-10 05:00 PST (History)
3 users (show)
ehsan: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (requires focus, requires browse-with-caret pref) (312 bytes, text/html)
2011-11-03 03:07 PDT, Jesse Ruderman
no flags Details
stack traces (11.71 KB, text/plain)
2011-11-03 03:07 PDT, Jesse Ruderman
no flags Details
Patch (v1) (1.30 KB, patch)
2012-01-19 11:20 PST, :Ehsan Akhgari (busy, don't ask for review please)
roc: review+
Details | Diff | Review
Test case (1.12 KB, patch)
2012-01-20 13:33 PST, :Ehsan Akhgari (busy, don't ask for review please)
roc: review+
Details | Diff | Review
Patch for landing (547 bytes, patch)
2012-01-26 13:41 PST, :Ehsan Akhgari (busy, don't ask for review please)
no flags Details | Diff | Review
Patch for landing (2.36 KB, patch)
2012-01-26 13:41 PST, :Ehsan Akhgari (busy, don't ask for review please)
no flags Details | Diff | Review
Fixed a bunch of test issues (3.12 KB, patch)
2012-01-31 13:31 PST, :Ehsan Akhgari (busy, don't ask for review please)
roc: review+
Details | Diff | Review

Description Jesse Ruderman 2011-11-03 03:07:08 PDT
Created attachment 571582 [details]
testcase (requires focus, requires browse-with-caret pref)

With
  user_pref("accessibility.browsewithcaret", true);
the testcase triggers:

###!!! ASSERTION: aOffset must be in the frame's range: 'aOffset >= contentOffset && aOffset <= contentOffset + contentLength', file layout/generic/nsTextFrameThebes.cpp, line 5122
Comment 1 Jesse Ruderman 2011-11-03 03:07:52 PDT
Created attachment 571583 [details]
stack traces
Comment 2 :Ehsan Akhgari (busy, don't ask for review please) 2012-01-16 19:22:28 PST
nsTextFrame::GetChildFrameContainingOffset will happily return offsets that are outside of a frame's offset range.  I don't know why, but is that something which should be fixed?
Comment 3 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-01-16 19:38:47 PST
That should only happen if the offset is beyond the text node length. Is that the situation here?
Comment 4 :Ehsan Akhgari (busy, don't ask for review please) 2012-01-17 15:36:48 PST
Yes.  aOffset is 2, the text node length is 1.
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-01-18 17:50:44 PST
I reckon we should leave that code as-is, assert that the offset is within the text node length, and fix any callers that aren't doing that.
Comment 6 :Ehsan Akhgari (busy, don't ask for review please) 2012-01-19 11:20:54 PST
Created attachment 589924 [details] [diff] [review]
Patch (v1)

Due to the fact that this test case requires caret browsing, I don't think there is a good way for us to import it as a crashtest...
Comment 7 Jesse Ruderman 2012-01-19 19:08:31 PST
> Due to the fact that this test case requires caret browsing, I don't think
> there is a good way for us to import it as a crashtest...

Try this in the crashtest manifest:

   pref(accessibility.browsewithcaret,true) load ...
Comment 8 :Ehsan Akhgari (busy, don't ask for review please) 2012-01-20 13:30:52 PST
(In reply to Jesse Ruderman from comment #7)
> > Due to the fact that this test case requires caret browsing, I don't think
> > there is a good way for us to import it as a crashtest...
> 
> Try this in the crashtest manifest:
> 
>    pref(accessibility.browsewithcaret,true) load ...

Neat!
Comment 9 :Ehsan Akhgari (busy, don't ask for review please) 2012-01-20 13:33:41 PST
Created attachment 590320 [details] [diff] [review]
Test case
Comment 10 :Ehsan Akhgari (busy, don't ask for review please) 2012-01-26 13:41:06 PST
Created attachment 591926 [details] [diff] [review]
Patch for landing
Comment 11 :Ehsan Akhgari (busy, don't ask for review please) 2012-01-26 13:41:37 PST
Created attachment 591927 [details] [diff] [review]
Patch for landing
Comment 12 Mozilla RelEng Bot 2012-01-26 14:38:19 PST
Autoland Patchset:
	Patches: 591927
	Branch: mozilla-central => try
	Destination: ssh://hg.mozilla.org/try
Try run started, revision 5ad14c5b0d1a. To cancel or monitor the job, see: https://build.mozilla.org/buildapi/self-serve/try/rev/5ad14c5b0d1a
Comment 13 Mozilla RelEng Bot 2012-01-26 19:45:22 PST
Try run for 5ad14c5b0d1a is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=5ad14c5b0d1a
Results (out of 208 total builds):
    exception: 2
    success: 174
    warnings: 32
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-5ad14c5b0d1a
Comment 14 :Ehsan Akhgari (busy, don't ask for review please) 2012-01-31 13:31:08 PST
Created attachment 593207 [details] [diff] [review]
Fixed a bunch of test issues

Only do the check for text frames, and also fix a test issue which was causing orange.
Comment 15 :Ehsan Akhgari (busy, don't ask for review please) 2012-02-09 13:40:39 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/7dcf885c2139
Comment 16 Ed Morley [:emorley] 2012-02-10 05:00:38 PST
https://hg.mozilla.org/mozilla-central/rev/7dcf885c2139

Note You need to log in before you can comment on or make changes to this bug.