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)
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)
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.
Reporter | ||
Comment 1•7 years ago
|
||
Reporter | ||
Comment 2•7 years ago
|
||
Updated•7 years ago
|
Component: Untriaged → Editor
Product: Firefox → Core
Updated•7 years ago
|
Assignee | ||
Comment 3•7 years ago
|
||
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
Assignee | ||
Comment 4•7 years ago
|
||
I'm not sure exact regression bug. However, this is caused by changing the default paragraph separator.
Blocks: 1430551
status-firefox59:
--- → unaffected
status-firefox60:
--- → affected
status-firefox61:
--- → affected
Keywords: regression
Assignee | ||
Comment 5•7 years ago
|
||
Assignee | ||
Comment 6•7 years ago
|
||
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
See Also: → https://github.com/w3c/editing/issues/175
Assignee | ||
Comment 7•7 years ago
|
||
Comment hidden (mozreview-request) |
Comment 9•7 years ago
|
||
mozreview-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/#review238154
Attachment #8963600 -
Flags: review?(m_kato) → review+
Comment 10•7 years ago
|
||
mozreview-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
Comment 11•7 years ago
|
||
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
Comment 12•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee | ||
Comment 13•7 years ago
|
||
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?
Updated•7 years ago
|
status-firefox-esr52:
--- → unaffected
Comment 14•7 years ago
|
||
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+
Comment 15•7 years ago
|
||
bugherder uplift |
Updated•7 years ago
|
Flags: in-testsuite+
Comment 16•7 years ago
|
||
(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.
Description
•