Closed Bug 1435123 Opened 7 years ago Closed 7 years ago

Pressing enter/return inside an inline editing host causes creating another inline editing host

Categories

(Core :: DOM: Editor, defect, P3)

60 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- fixed
firefox61 --- fixed

People

(Reporter: huijing, Assigned: masayuki)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(4 files)

Attached video contenteditable.mp4 (deleted) —
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0) Gecko/20100101 Firefox/60.0 Build ID: 20180201220225 Steps to reproduce: 1. Have a style tag set to display: block and contenteditable="true" 2. Press enter/return while caret is active Actual results: The style element was duplicated. Expected results: There should have been a line break as per previous editions of Firefox.
Attached image contenteditable.gif (deleted) —
Attached file testcase-contenteditable.html (deleted) —
Component: Untriaged → Editor
Product: Firefox → Core
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3
Oh, this must be serious regression. I don't remember the bug#, IIRC, I fixed same bug...
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
OS: Unspecified → All
Hardware: Unspecified → All
Summary: Pressing enter/return inside a contenteditable style tag generates a new div → Pressing enter/return inside an inline editing host causes creating another inline editing host
I'm not sure exact regression bug. However, this is caused by changing the default paragraph separator.
Hmm, I think that all vendors can agree with inserting <br> element in inline editing host. However, it shouldn't be included into WPT for now since no documents declares this behavior and even Chrome/Safari doesn't work well in insertParagraph.html. Filed spec bug: https://github.com/w3c/editing/issues/175
Comment on attachment 8963600 [details] Bug 1435123 - HTMLEditor::GetBlock() has to specify given ancestor limit node to HTMLEditor::GetBlockNodeParent() https://reviewboard.mozilla.org/r/232498/#review238154
Attachment #8963600 - Flags: review?(m_kato) → review+
Comment on attachment 8963600 [details] Bug 1435123 - HTMLEditor::GetBlock() has to specify given ancestor limit node to HTMLEditor::GetBlockNodeParent() https://reviewboard.mozilla.org/r/232498/#review238158
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/d7fa5324198f HTMLEditor::GetBlock() has to specify given ancestor limit node to HTMLEditor::GetBlockNodeParent() r=m_kato
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Comment on attachment 8963600 [details] Bug 1435123 - HTMLEditor::GetBlock() has to specify given ancestor limit node to HTMLEditor::GetBlockNodeParent() Approval Request Comment [Feature/Bug causing the regression]: Regression of bug 1430551 (from point of view of release channel). [User impact if declined]: May duplicate editing host (i.e., <foo contenteditable>) unexpectedly when typing Enter key. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No since automated tests completely tests possible simple cases. [List of other uplifts needed for the feature/fix]: No. [Is the change risky?]: No. [Why is the change risky/not risky?]: HTMLEditor::GetBlock() is an internal API of HTMLEditor. However, the second argument, which is the cause of this bug, is not used by the other callers (i.e., called only by HTMLEditRules::WillInsertBreak()): https://searchfox.org/mozilla-central/search?q=symbol:_ZN7mozilla10HTMLEditor8GetBlockER7nsINodePS1_&redirect=false Therefore, this patch actually changes only the path handling Enter key press. [String changes made/needed]: No.
Attachment #8963600 - Flags: approval-mozilla-beta?
Comment on attachment 8963600 [details] Bug 1435123 - HTMLEditor::GetBlock() has to specify given ancestor limit node to HTMLEditor::GetBlockNodeParent() One-line fix with an automated test to fix a new regression in Fx60 (from the release point of view). Approved for 60.0b9.
Attachment #8963600 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: in-testsuite+
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #13) > [Is this code covered by automated tests?]: > Yes. > > [Has the fix been verified in Nightly?]: > Yes. > > [Needs manual test from QE? If yes, steps to reproduce]: > No since automated tests completely tests possible simple cases. Setting qe-verify- based on Masayuki Nakano's assessment on manual testing needs and the fact that this fix has automated tests.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: