Closed
Bug 1397412
Opened 7 years ago
Closed 7 years ago
Implement Mochitest to ensure functioning of plaintext editor's EditorBase::FindBetterInsertionPoint()
Categories
(Core :: DOM: Editor, enhancement, P3)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
mozilla57
| Tracking | Status | |
|---|---|---|
| firefox57 | --- | fixed |
People
(Reporter: jorgk-bmo, Assigned: jorgk-bmo)
References
Details
Attachments
(1 file, 2 obsolete files)
|
3.51 KB,
patch
|
jorgk-bmo
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1393140 +++
|
Assignee | |
Comment 1•7 years ago
|
||
This test fails with attachment 8903033 [details] [diff] [review] from bug 1393140 applied. In fact, once the test is finished, you can try typing into the empty line between the xx and the yy to see where the character is inserted :-(
Attachment #8905278 -
Flags: review?(ehsan)
|
|
Assignee | |
Comment 2•7 years ago
|
||
Comment on attachment 8905278 [details] [diff] [review] 1397412-FindBetterInsertionPoint.patch (v1) Review of attachment 8905278 [details] [diff] [review]: ----------------------------------------------------------------- ::: editor/libeditor/tests/test_bug1397412.html @@ +1,4 @@ > +<!DOCTYPE HTML> > +<html> > +<!-- > +https://bugzilla.mozilla.org/show_bug.cgi?id=1330796 Oops, copy/paste error here. I'll fix it in v2.
|
||
Comment 3•7 years ago
|
||
Comment on attachment 8905278 [details] [diff] [review] 1397412-FindBetterInsertionPoint.patch (v1) Review of attachment 8905278 [details] [diff] [review]: ----------------------------------------------------------------- The test logic is mostly correct but see the comment below; it needs to be rewritten as a XUL test. I'd like to look at a modified version if possible. Thanks. ::: editor/libeditor/tests/test_bug1397412.html @@ +48,5 @@ > + .getInterface(Ci.nsIWebNavigation) > + .QueryInterface(Ci.nsIInterfaceRequestor) > + .getInterface(Ci.nsIEditingSession); > + var editor = editingSession.getEditorForWindow(window); > + editor.flags |= Ci.nsIPlaintextEditor.eEditorPlaintextMask; This is a rather made-up mode for the editor, in practice it will never work in this setup. The code path you are interested in is <https://searchfox.org/mozilla-central/rev/67f38de2443e6b613d874fcf4d2cd1f2fc3d5e97/editor/composer/nsEditingSession.cpp#356> (or line 360) which you can get by using a XUL <editor> element with editortype="textmail" or "text" respectively. You can use this test as an example <https://searchfox.org/mozilla-central/rev/67f38de2443e6b613d874fcf4d2cd1f2fc3d5e97/editor/libeditor/tests/test_bug489202.xul#20> (there's a lot of boilerplate that you can copy from that test. You can create a <div> or some such under the editor element there and drop the |xx<br><br>yy<br>| content inside it. The core test logic here can be ported almost directly to that setup, but it's better to use a more real editor setup to make sure what the test exercises has some resemblance to something that the code can encounter in practice.
Attachment #8905278 -
Flags: review?(ehsan)
|
|
Assignee | |
Comment 4•7 years ago
|
||
The XUL version turned out to be much nicer and allowed me to do two tests easily. A failure looks like this: 2 INFO TEST-UNEXPECTED-FAIL | editor/libeditor/tests/test_bug1397412.xul | body.innerHTML: got |xxt<br><br>|, expected |xx<br>t<br>| 3 INFO TEST-UNEXPECTED-FAIL | editor/libeditor/tests/test_bug1397412.xul | body.innerHTML: got |xx<br><br>yyt<br>|, expected |xx<br>t<br>yy<br>| A pass looks like this: 2 INFO TEST-PASS | editor/libeditor/tests/test_bug1397412.xul | body.innerHTML: got |xx<br>t<br>|, expected |xx<br>t<br>| 3 INFO TEST-PASS | editor/libeditor/tests/test_bug1397412.xul | body.innerHTML: got |xx<br>t<br>yy<br>|, expected |xx<br>t<br>yy<br>|
Attachment #8905278 -
Attachment is obsolete: true
Attachment #8905406 -
Flags: review?(ehsan)
|
|
||
Comment 5•7 years ago
|
||
Comment on attachment 8905406 [details] [diff] [review] 1397412-FindBetterInsertionPoint.patch (v2). Review of attachment 8905406 [details] [diff] [review]: ----------------------------------------------------------------- ::: editor/libeditor/tests/test_bug1397412.xul @@ +53,5 @@ > + body.innerHTML = initialHTML1; > + selection.collapse(body, 2); > + synthesizeKey("t", { code: "KeyT" }); > + var actualHTML = body.innerHTML; > + ok(actualHTML == expectedHTML1, Use is(), not ok(). @@ +61,5 @@ > + body.innerHTML = initialHTML2; > + selection.collapse(body, 2); > + synthesizeKey("t", { code: "KeyT" }); > + actualHTML = body.innerHTML; > + ok(actualHTML == expectedHTML2, Ditto.
Attachment #8905406 -
Flags: review?(ehsan) → review+
|
|
Assignee | |
Comment 6•7 years ago
|
||
Changed ok() to is(). Carrying forward Ehsan's r+. https://treeherder.mozilla.org/#/jobs?repo=try&revision=b515e9e2d5502a710e561f72af7ea3bad544144b
Attachment #8905406 -
Attachment is obsolete: true
Attachment #8905642 -
Flags: review+
|
|
Assignee | |
Updated•7 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/903bd3c9bfdd Implement Mochitest for EditorBase::FindBetterInsertionPoint() in plaintext editor. r=ehsan
Keywords: checkin-needed
|
||
Comment 8•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/903bd3c9bfdd
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•