Closed
Bug 776323
Opened 9 years ago
Closed 9 years ago
Crash with inserthtml, strikethrough on img
Categories
(Core :: DOM: Editor, defect)
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: jruderman, Assigned: ayg)
References
(Blocks 1 open bug)
Details
(Keywords: crash, regression, testcase)
Crash Data
Attachments
(4 files)
404 bytes,
text/html
|
Details | |
14.29 KB,
text/plain
|
Details | |
2.09 KB,
patch
|
ehsan
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
bajaj
:
approval-mozilla-esr10+
|
Details | Diff | Splinter Review |
35.02 KB,
patch
|
ehsan
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•9 years ago
|
||
Nightly: bp-fcc805ed-36e7-47d7-93da-2765e2120722
Comment 2•9 years ago
|
||
On Windows: bp-72bb4d49-3eb8-44d2-8482-d64272120722.
Crash Signature: [@ nsCOMPtr_base::begin_assignment | nsSelectionState::SaveSelection ]
[@ nsRangeStore::StoreRange | nsSelectionState::SaveSelection] → [@ nsCOMPtr_base::begin_assignment | nsSelectionState::SaveSelection ]
[@ nsRangeStore::StoreRange | nsSelectionState::SaveSelection]
[@ nsCOMPtr_base::assign_assuming_AddRef(nsISupports*) | nsRangeStore::StoreRange(nsIDOMRange*)]
OS: Mac OS X → All
Hardware: x86_64 → All
Assignee | ||
Comment 3•9 years ago
|
||
This was a stupid regression from bug 771994. Easy to fix, though.
Assignee: nobody → ayg
Blocks: CVE-2012-3959
Status: NEW → ASSIGNED
status-firefox16:
--- → affected
status-firefox17:
--- → affected
Keywords: regression
Updated•9 years ago
|
Version: Trunk → 16 Branch
Assignee | ||
Comment 4•9 years ago
|
||
So I blame the fact that C for loops are used for both "repeat X times" and "do something to each thing in a range". If we had a construct like "repeat(count)", with no unused index variable, this kind of bug would be impossible. Also, I blame multi-range selections. If selections had only one range, this kind of bug would also be impossible. There would be no loop here to start with. Alternatively, I should pay attention when I code rather than finding things to blame for my bugs, but that's less fun. [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 771994 User impact if declined: Crash due to null pointer dereference, already observed in the wild. Testing completed (on m-c, etc.): None yet. Risk to taking this patch (and alternatives if risky): Low. The fix is effectively one line. The old code was just completely broken, and nobody noticed right away because we have lousy test coverage for multi-range selections. String or UUID changes made by this patch: None.
Attachment #644741 -
Flags: review?(ehsan)
Attachment #644741 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 5•9 years ago
|
||
Cleanup patch after the actual bug fix this time, for ease of backporting. This cleanup spilled over into requiring lots of de-COMtamination, just the way I like it! Try: https://tbpl.mozilla.org/?tree=Try&rev=d08869e2b5c4
Attachment #644742 -
Flags: review?(ehsan)
Updated•9 years ago
|
Attachment #644741 -
Flags: review?(ehsan) → review+
Updated•9 years ago
|
Attachment #644742 -
Flags: review?(ehsan) → review+
Assignee | ||
Comment 6•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/12f5569247f0 https://hg.mozilla.org/integration/mozilla-inbound/rev/51a7bf2b46f1
Flags: in-testsuite+
Target Milestone: --- → mozilla17
Comment 7•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/12f5569247f0 https://hg.mozilla.org/mozilla-central/rev/51a7bf2b46f1
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
status-firefox17:
affected → ---
Comment 8•9 years ago
|
||
Comment on attachment 644741 [details] [diff] [review] Patch part 1 -- Fix crash due to silly logic error in nsSelectionState::SaveSelection This is needed on beta as well.
Attachment #644741 -
Flags: approval-mozilla-beta?
Updated•9 years ago
|
tracking-firefox16:
--- → +
Comment 9•9 years ago
|
||
(In reply to :Aryeh Gregor from comment #4) > Alternatively, I should pay attention when I code rather than finding things > to blame for my bugs, but that's less fun. :) > [Approval Request Comment] > Risk to taking this patch (and alternatives if risky): Low. The fix is > effectively one line. The old code was just completely broken, and nobody > noticed right away because we have lousy test coverage for multi-range > selections. Sounds good, let's get this on branches.
Updated•9 years ago
|
status-firefox15:
--- → affected
tracking-firefox15:
--- → +
Updated•9 years ago
|
Attachment #644741 -
Flags: approval-mozilla-beta?
Attachment #644741 -
Flags: approval-mozilla-beta+
Attachment #644741 -
Flags: approval-mozilla-aurora?
Attachment #644741 -
Flags: approval-mozilla-aurora+
Comment 10•9 years ago
|
||
http://hg.mozilla.org/releases/mozilla-aurora/rev/ab0ab9e65f96 http://hg.mozilla.org/releases/mozilla-beta/rev/80000a1e4ea0
Comment 11•9 years ago
|
||
Verified that Firefox 15 beta 3 does not crash when loading the test case from the Description. Verified on Windows 7, Ubuntu 12.04 and Mac OS X 10.6: Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20100101 Firefox/15.0 Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20100101 Firefox/15.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:15.0) Gecko/20100101 Firefox/15.0 Verified also the Socorro reports and could not find any crashes that occurred after the patches landed.
Comment 13•9 years ago
|
||
It's #3 top browser crasher in 10.0.7esr. Did bug 771994 land in this version?
Updated•9 years ago
|
Comment 14•9 years ago
|
||
Comment on attachment 644741 [details] [diff] [review] Patch part 1 -- Fix crash due to silly logic error in nsSelectionState::SaveSelection See the risk assessment for the previous branch approval requests. The short version is that this is pretty safe.
Attachment #644741 -
Flags: approval-mozilla-esr10?
Comment 15•9 years ago
|
||
Comment on attachment 644741 [details] [diff] [review] Patch part 1 -- Fix crash due to silly logic error in nsSelectionState::SaveSelection Approving the patch as it is low risk and needed for ESR .
Attachment #644741 -
Flags: approval-mozilla-esr10? → approval-mozilla-esr10+
Comment 17•9 years ago
|
||
Verified that Firefox 10.0.8 ESR does not crash when loading the test case from the Description. Verified on Windows 7, Ubuntu 12.04 and Mac OS X 10.7: Mozilla/5.0 (Windows NT 6.1; rv:10.0.8) Gecko/20100101 Firefox/10.0.8 Mozilla/5.0 (X11; Linux i686; rv:10.0.8) Gecko/20100101 Firefox/10.0.8 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:10.0.8) Gecko/20100101 Firefox/10.0.8
You need to log in
before you can comment on or make changes to this bug.
Description
•