Closed
Bug 1130651
Opened 9 years ago
Closed 7 years ago
`document.execCommand('insertText', false, '\n')` with `contentEditable` element throws `IndexSizeError`.
Categories
(Core :: DOM: Editor, defect)
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: mikol, Assigned: ayg)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.94 Safari/537.36 Steps to reproduce: 1. Open index-size-error.html. 2. Type something. For example: “This is a test.” 3. Position the text caret in the middle of the string you typed after a space, immediately before a character. For example: “This is |a test.” 4. Press the Return/Enter key to insert a line break. Actual results: The following error will be thrown: “IndexSizeError: Index or size is negative or greater than the allowed amount”. The operation that causes the error (`document.execCommand('insertText', false, '\n')`) actually succeeds so it’s not clear why it throws an error after it succeeds but before it returns. Expected results: No error should have been thrown.
Comment 1•9 years ago
|
||
Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=02566635767f&tochange=1533eef79502 Suspect: Bug 1088054 and Bug 1086349
Status: UNCONFIRMED → NEW
status-firefox35:
--- → unaffected
status-firefox36:
--- → affected
status-firefox37:
--- → affected
status-firefox38:
--- → affected
status-firefox-esr31:
--- → unaffected
Component: Untriaged → Editor
Ever confirmed: true
Flags: needinfo?(ayg)
Keywords: regression
OS: Mac OS X → All
Product: Firefox → Core
Version: 33 Branch → 36 Branch
Comment 2•9 years ago
|
||
wait Please ignote comment #1
Updated•9 years ago
|
Updated•9 years ago
|
status-firefox35:
affected → ---
status-firefox36:
affected → ---
status-firefox37:
affected → ---
status-firefox38:
affected → ---
status-firefox-esr31:
affected → ---
Comment 3•9 years ago
|
||
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bcb614c5e7fa&tochange=cd62c4b8f500 The error is thrown since landing Bug 748307. So, this is not a regression.
Blocks: 748307
Version: 33 Branch → 16 Branch
Assignee | ||
Comment 4•9 years ago
|
||
It doesn't entirely surprise me that this is broken, because it's an edge case. The correct behavior should be identical to document.execCommand('insertparagraph'), I think, but it would be good to check against WebKit/Blink. Tests should include inside a block (like a <p> or list item) to make sure we're doing insertparagraph and not insertlinebreak. And obviously, it shouldn't throw.
Comment 5•8 years ago
|
||
also, an empty string with insertText throws NS_ERROR_NOT_IMPLEMENTED Same problem affected inserthtml Bug 309731
Assignee | ||
Updated•7 years ago
|
Blocks: editor-blink-compat
Assignee | ||
Comment 6•7 years ago
|
||
More minimal test-case: data:text/html,<!doctype html> <div contenteditable>a b</div> <script> var div = document.querySelector("div"); div.focus(); getSelection().collapse(div.firstChild, 2); try { document.execCommand("inserttext", false, "\n"); document.body.textContent = "success"; } catch(e) { document.body.textContent = e; } </script>
Assignee: nobody → ayg
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
I opened bug 1352144 for the empty-string issue.
Assignee | ||
Updated•7 years ago
|
Attachment #8853012 -
Flags: review?(masayuki)
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8853012 [details] Bug 1130651 - Don't throw IndexSizeError from insertText command https://reviewboard.mozilla.org/r/125142/#review128358 ::: editor/libeditor/HTMLEditRules.cpp:1417 (Diff revision 1) > else if (subStr.Equals(newlineStr)) { > + NS_ENSURE_STATE(mHTMLEditor); > + AutoTrackDOMPoint tracker(mHTMLEditor->mRangeUpdater, > + address_of(selNode), &selOffset); > nsCOMPtr<Element> br = wsObj.InsertBreak(address_of(curNode), > &curOffset, > nsIEditor::eNone); I don't understand why does HTMLEditRules::WillInsertText() need to track selection range during an insertion only in this case. What's the difference from the other cases (especially, from |if (isPRE || IsPlaintextEditor())| cases)? Could you explain the reason in the commit message?
Assignee | ||
Comment 10•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8853012 [details] Bug 1130651 - Don't throw IndexSizeError from insertText command https://reviewboard.mozilla.org/r/125142/#review128358 > I don't understand why does HTMLEditRules::WillInsertText() need to track selection range during an insertion only in this case. What's the difference from the other cases (especially, from |if (isPRE || IsPlaintextEditor())| cases)? > > Could you explain the reason in the commit message? Yes, you're right, my fix should have been more general.
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
mozreview-review |
Comment on attachment 8853012 [details] Bug 1130651 - Don't throw IndexSizeError from insertText command https://reviewboard.mozilla.org/r/125142/#review128506
Attachment #8853012 -
Flags: review?(masayuki) → review+
Comment 13•7 years ago
|
||
Pushed by ayg@aryeh.name: https://hg.mozilla.org/integration/autoland/rev/0aeb3c980745 Don't throw IndexSizeError from insertText command r=masayuki
Assignee | ||
Comment 14•7 years ago
|
||
Oops, I got this confused with bug 1352144 and landed a fixed version. If you're not happy with the fixed version, please back out and let me know what to change. (You gave r+, but it seems like you just wanted me to explain my reason and not change the code, and I changed the code instead.)
Flags: needinfo?(masayuki)
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0aeb3c980745
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•