1408227 - Correctly update the child at offset pointer across calls to WSRunObject::InsertBreak()

Status: RESOLVED FIXED

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

Comment 1

[(no longer active)]

Attachment: Correctly update the child at offset pointer across calls to WSRunObject::InsertBreak()

Comment 2

[(no longer active)]

This patch fixes the problem I talked about in bug 1407966 comment 9.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Comment on attachment 8918064 [details] [diff] [review]
Correctly update the child at offset pointer across calls to WSRunObject::InsertBreak()

> nsresult
> TextEditor::CreateBRImpl(nsCOMPtr<nsIDOMNode>* aInOutParent,
>                          int32_t* aInOutOffset,
>+                         nsCOMPtr<nsIContent>* aInOutChildAtOffset,
>                          nsCOMPtr<nsIDOMNode>* outBRNode,
>                          EDirection aSelect)
> {
>   NS_ENSURE_TRUE(aInOutParent && *aInOutParent && aInOutOffset && outBRNode, NS_ERROR_NULL_POINTER);
>   *outBRNode = nullptr;
> 
>   // we need to insert a br.  unfortunately, we may have to split a text node to do it.
>   nsCOMPtr<nsINode> node = do_QueryInterface(*aInOutParent);
>   int32_t theOffset = *aInOutOffset;
>+  int32_t offsetChange = 1;

Really difficult to understand what this 1 means. Please set 0 to here since and set it when we change *aInOutChildAtOffset.

Or, I like this way is better, declare |const int32_t kInOffset = *aInOutOffset;|. Then, you can check the difference with |*aInOutChildAtOffset - kInOffset| at the new for loop.

>@@ -460,24 +464,30 @@ TextEditor::CreateBRImpl(nsCOMPtr<nsIDOMNode>* aInOutParent,
>       tmp = GetNodeLocation(node, &offset);
>     }
>     // create br
>     brNode = CreateNode(nsGkAtoms::br, tmp, offset);
>     if (NS_WARN_IF(!brNode)) {
>       return NS_ERROR_FAILURE;
>     }
>     *aInOutParent = GetAsDOMNode(tmp);
>+    offsetChange = offset + 2 - *aInOutOffset; // (end - start) + 1

With your comment, I understand this as:

(OutOffset - InOffset) + 1.

So, I like |(offset + 1) - *aInOutOffset + 1|.

However, I don't understand the last |+ 1|. Could you explain it with comment? Or please consider to use kInOffset.

>     *aInOutOffset = offset+1;
>   } else {
>     brNode = CreateNode(nsGkAtoms::br, node, theOffset);
>     if (NS_WARN_IF(!brNode)) {
>       return NS_ERROR_FAILURE;
>     }
>     (*aInOutOffset)++;

Please set offsetChange to 1 here or do nothing if you use kInOffset.

>+  for (int32_t i = 0; i < offsetChange; ++i) {

So, I guess this can be:

  for (int32_t i = 0; i < *aInOutChildAtOffset - kInOffset; ++i) {

Otherwise, looks okay to me. Although, I'd like to check the new patch in usual schedule. However, I'll go away starting tomorrow. So, giving r+ for making you can land new patch.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/febed94fa592
Correctly update the child at offset pointer across calls to WSRunObject::InsertBreak(); r=masayuki

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for asserting at editor/libeditor/EditorBase.cpp:2513 during execution of web-platform-test /editing/run/inserttext.html:

https://hg.mozilla.org/integration/mozilla-inbound/rev/763378dbc0035a1cbe81270d16a088e9d3569ca6

Push which ran failing test: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=508179b0dad5311979a135b9a03721da53ca868b&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=136827503&repo=mozilla-inbound

[task 2017-10-13T15:28:07.400Z] 15:28:07     INFO - TEST-START | /editing/run/inserttext.html
[task 2017-10-13T15:28:07.456Z] 15:28:07     INFO - PID 3041 | [Child 3091, Main Thread] WARNING: NS_ENSURE_TRUE(mDoneSetup) failed: file /builds/worker/workspace/build/src/editor/composer/nsEditingSession.cpp, line 1284
[task 2017-10-13T15:28:07.513Z] 15:28:07     INFO - PID 3041 | ++DOCSHELL 0x7fba40319000 == 4 [pid = 3091] [id = {991651aa-3d88-4ee9-8419-2dac3f4fa960}]
[task 2017-10-13T15:28:07.514Z] 15:28:07     INFO - PID 3041 | ++DOMWINDOW == 10 (0x7fba40319800) [pid = 3091] [serial = 65] [outer = (nil)]
[task 2017-10-13T15:28:07.529Z] 15:28:07     INFO - PID 3041 | [Parent 3041, Main Thread] WARNING: [nsFrameLoader] ReallyStartLoadingInternal tried but couldn't show remote browser.
[task 2017-10-13T15:28:07.530Z] 15:28:07     INFO - PID 3041 | : file /builds/worker/workspace/build/src/dom/base/nsFrameLoader.cpp, line 904
[task 2017-10-13T15:28:07.538Z] 15:28:07     INFO - PID 3041 | ++DOMWINDOW == 11 (0x7fba4031a800) [pid = 3091] [serial = 66] [outer = 0x7fba40319800]
[task 2017-10-13T15:28:07.607Z] 15:28:07     INFO - PID 3041 | 1507908487600	Marionette	DEBUG	Register listener.js for window 2147483713
[task 2017-10-13T15:28:07.689Z] 15:28:07     INFO - PID 3041 | ++DOMWINDOW == 12 (0x7fba405ab000) [pid = 3091] [serial = 67] [outer = 0x7fba40319800]
[task 2017-10-13T15:28:07.788Z] 15:28:07     INFO - PID 3041 | [Child 3091, Main Thread] WARNING: stylo: Web Components not supported yet: file /builds/worker/workspace/build/src/dom/base/nsDocument.cpp, line 6457
[task 2017-10-13T15:28:07.788Z] 15:28:07     INFO - PID 3041 | [Child 3091, Main Thread] WARNING: stylo: Web Components not supported yet: file /builds/worker/workspace/build/src/dom/base/nsDocument.cpp, line 6457
[task 2017-10-13T15:28:08.111Z] 15:28:08     INFO - PID 3041 | Assertion failure: node->GetChildAt(offset) == *aInOutChildAtOffset, at /builds/worker/workspace/build/src/editor/libeditor/EditorBase.cpp:2513
[task 2017-10-13T15:28:08.329Z] 15:28:08     INFO - PID 3041 | #01: mozilla::WSRunObject::InsertText(nsTSubstring<char16_t> const&, nsCOMPtr<nsINode>*, nsCOMPtr<nsIContent>*, int*, nsIDocument*) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.333Z] 15:28:08     INFO - PID 3041 | #02: mozilla::HTMLEditRules::WillInsertText(EditAction, mozilla::dom::Selection*, bool*, bool*, nsTSubstring<char16_t> const*, nsTSubstring<char16_t>*, int) [clone .part.427] (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.342Z] 15:28:08     INFO - PID 3041 | #03: mozilla::HTMLEditRules::WillDoAction(mozilla::dom::Selection*, mozilla::RulesInfo*, bool*, bool*) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.351Z] 15:28:08     INFO - PID 3041 | #04: mozilla::TextEditor::InsertText(nsTSubstring<char16_t> const&) [clone .part.180] (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.359Z] 15:28:08     INFO - PID 3041 | #05: mozilla::InsertPlaintextCommand::DoCommandParams(char const*, nsICommandParams*, nsISupports*) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.363Z] 15:28:08     INFO - PID 3041 | #06: nsControllerCommandTable::DoCommandParams(char const*, nsICommandParams*, nsISupports*) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.372Z] 15:28:08     INFO - PID 3041 | #07: nsBaseCommandController::DoCommandWithParams(char const*, nsICommandParams*) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.380Z] 15:28:08     INFO - PID 3041 | #08: nsCommandManager::DoCommand(char const*, nsICommandParams*, mozIDOMWindowProxy*) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.384Z] 15:28:08     INFO - PID 3041 | #09: nsHTMLDocument::ExecCommand(nsTSubstring<char16_t> const&, bool, nsTSubstring<char16_t> const&, nsIPrincipal&, mozilla::ErrorResult&) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.393Z] 15:28:08     INFO - PID 3041 | #10: mozilla::dom::HTMLDocumentBinding::execCommand(JSContext*, JS::Handle<JSObject*>, nsHTMLDocument*, JSJitMethodCallArgs const&) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.397Z] 15:28:08     INFO - PID 3041 | #11: mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.405Z] 15:28:08     INFO - PID 3041 | #12: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.409Z] 15:28:08     INFO - PID 3041 | #13: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.417Z] 15:28:08     INFO - PID 3041 | #14: InternalCall(JSContext*, js::AnyInvokeArgs const&) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.422Z] 15:28:08     INFO - PID 3041 | #15: Interpret(JSContext*, js::RunState&) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.430Z] 15:28:08     INFO - PID 3041 | #16: js::RunScript(JSContext*, js::RunState&) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.434Z] 15:28:08     INFO - PID 3041 | #17: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.435Z] 15:28:08     INFO - PID 3041 | #18: InternalCall(JSContext*, js::AnyInvokeArgs const&) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.443Z] 15:28:08     INFO - PID 3041 | #19: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.451Z] 15:28:08     INFO - PID 3041 | #20: js::fun_apply(JSContext*, unsigned int, JS::Value*) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.451Z] 15:28:08     INFO - PID 3041 | #21: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.452Z] 15:28:08     INFO - PID 3041 | #22: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.452Z] 15:28:08     INFO - PID 3041 | #23: InternalCall(JSContext*, js::AnyInvokeArgs const&) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.456Z] 15:28:08     INFO - PID 3041 | #24: js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) (/builds/worker/workspace/build/application/firefox/libxul.so)
[task 2017-10-13T15:28:08.456Z] 15:28:08     INFO - PID 3041 | #25: ??? (???:???)

Comment 6

[(no longer active)]

This bug needs an owner to debug the test failure and reland this...

Comment 7

[Jorg K (CEST = GMT+2)]

Note that Assertion failure: node->GetChildAt(offset) == *aInOutChildAtOffset is the summary of bug 1407966 from which we got here. It's all caused by bug 1406482 whose backout I've requested a few times.

Comment 8

[Jorg K (CEST = GMT+2)]

Just out of interest: How important is this bug? It's a regression of bug 1406482 but what are the symptoms now that bug 1407966 is fixed?

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Currently (with patches under review), editor code is always validated or computes offset properly. So, this must not be a regression for now but we need to redesign Insert*() methods with Editor(Raw)DOMPoint.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd76df85ce1f76fb850f69e3719f90c6c8842e44

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9cfc5735603446b19104e4940955546bf5e989e5

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e361f6ca01246b818569b361f4889e6304fecb9

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8409b1949ee557f1711b52b5e5e168ec14fe923

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=09225eb97b8fa094e7c0bda2c5d6cafdcb6ec46d

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Attachment: Bug 1408227 - part 1: TextEditor::CreateBRImpl() should take |const EditorRawDOMPoint&| for insertion point of new <br> element

TextEditor::CreateBRImpl() should take |const EditorRawDOMPoint&| as insertion
point of new <br> element.  Additionally, it doesn't need to have out argument
for the point of after <br> element because callers can get it with
EditorRawDOMPoint(nsINode*) and the new <br> node, and calling AdvanceOffset().
This cost must be enough cheap.

Review commit: https://reviewboard.mozilla.org/r/201914/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/201914/

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Attachment: Bug 1408227 - part 2: WSRunObject::InsertBreak() should take |const EditorRawDOMPoint&| as an argument

WSRunObject::InsertBreak() should take |const EditorRawDOMPoint&| instead of
a pair of container node and offset in it.

Review commit: https://reviewboard.mozilla.org/r/201916/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/201916/

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Attachment: Bug 1408227 - part 3: Redesign WSRunObject::FindRun() with EditorRawDOMPoint

WSRunObject::FindRun() finds the nearest run from aPoint to specified direction.
So, it uses nsContentUtils::ComparePoints() a lot.  Therefore, it should use
an overload which takes RawRangeBoundary.  Although, it's not optimized for
RawRangeBoundary, but if it'd be optimized, this method becomes faster.

And this patch renames it to FindNearestRun().

Review commit: https://reviewboard.mozilla.org/r/201918/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/201918/

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Attachment: Bug 1408227 - part 4: WSRunObject::DeleteChars() should take two |const EditorRawDOMPoint&| arguments to specify a range to remove

WSRunObject::DeleteChars() should take two |const EditorRawDOMPoint&| arguments
to represent a range to remove.

Additionally, this renames it to DeleteRange() because it also removes any
nodes in the range.  So, "Chars" isn't good word for this method's name.

Review commit: https://reviewboard.mozilla.org/r/201920/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/201920/

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Attachment: Bug 1408227 - part 5: Redesign GetCharAfter(), GetCharBefore(), GetWSPointAfter() and GetWSPointBefore() of WSRunObject

WSRunObject::GetChar(After|Before)(nsINode*, int32_t) returns next/previous
character's DOM point as WSPoint.  If the container node is a text node in
mNodeArray, it calls WSRunObject::GetChar(After|Before)(const WSPoint&) which
returns next/previous offset if the point isn't end/start of the text node.
If the point is at end/start of the text node, it returns start/end of
next/previous text node in mNodeArray.  If the container node is not a text
node in mNodeArray, it calls WSRunObject::GetWSPoint(After|Before)().  It looks
for next/previous text node in mNodeArray from the point.  Then, it calls
WSRunObject::GetChar(After|Before)(const WSPoint&) and returns the result.

So, we should rename GetCharAfter() to GetNextCharPoint(), GetCharBefore() to
GetPreviousCharPoint(), GetWSPointAfter() to GetNextCharPointInternal() and
GetWSPointBefore() to GetPreviousCharPointInternal().

Then, they should take |const EditorRawDOMPoint&| instead of a set of container
node and offset in it.

So, looks like that "WS"RunObject is not good name for this class, perhaps,
AutoTextRunManager or something? But I'm still not sure.

Review commit: https://reviewboard.mozilla.org/r/201922/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/201922/

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Attachment: Bug 1408227 - part 6: WSRunObject::CheckTrailingNBSP() should take |const EditorRawDOMPoint&| instead of a set of container node and offset in it

The name, WSRunObject::CheckTrailingNBSP(), is unclear what it does.  It
replaces previous character of specified point with ASCII space if it is NBSP
and it's not necessary.

So, it should be renamed to ReplacePreviousNBSPIfUnncessary().

Additionally, it should take |const EditorRawDOMPoint&| instead of a set of
container node and offset in it.

Review commit: https://reviewboard.mozilla.org/r/201924/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/201924/

Comment 21

[Code Review Bot [:reviewbot]]

Comment on attachment 8933368 [details]
Bug 1408227 - part 2: WSRunObject::InsertBreak() should take |const EditorRawDOMPoint&| as an argument

https://reviewboard.mozilla.org/r/201916/#review209874


C/C++ static analysis found 1 defect in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`


::: editor/libeditor/HTMLEditRules.cpp:1896
(Diff revision 1)
> +    EditorRawDOMPoint afterBRNode(brNode);
> +    DebugOnly<bool> advanced = afterBRNode.AdvanceOffset();
> +    NS_WARNING_ASSERTION(advanced,
> +      "Failed to advance offset after the new <br> node");
> +    node = afterBRNode.Container();
> +    aOffset = afterBRNode.Offset();

Warning: Value stored to 'aoffset' is never read [clang-tidy: clang-analyzer-deadcode.DeadStores]

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0b5868929733077ae57bc84748f65901928a982

Comment 23

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Comment on attachment 8933368 [details]
Bug 1408227 - part 2: WSRunObject::InsertBreak() should take |const EditorRawDOMPoint&| as an argument

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/201916/diff/1-2/

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Comment on attachment 8933369 [details]
Bug 1408227 - part 3: Redesign WSRunObject::FindRun() with EditorRawDOMPoint

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/201918/diff/1-2/

Comment 25

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Comment on attachment 8933370 [details]
Bug 1408227 - part 4: WSRunObject::DeleteChars() should take two |const EditorRawDOMPoint&| arguments to specify a range to remove

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/201920/diff/1-2/

Comment 26

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Comment on attachment 8933371 [details]
Bug 1408227 - part 5: Redesign GetCharAfter(), GetCharBefore(), GetWSPointAfter() and GetWSPointBefore() of WSRunObject

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/201922/diff/1-2/

Comment 27

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(still sick)]

Comment on attachment 8933372 [details]
Bug 1408227 - part 6: WSRunObject::CheckTrailingNBSP() should take |const EditorRawDOMPoint&| instead of a set of container node and offset in it

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/201924/diff/1-2/

Comment 28

[Makoto Kato [:m_kato]]

Comment on attachment 8933367 [details]
Bug 1408227 - part 1: TextEditor::CreateBRImpl() should take |const EditorRawDOMPoint&| for insertion point of new <br> element

https://reviewboard.mozilla.org/r/201914/#review210316

Comment 29

[Makoto Kato [:m_kato]]

Comment on attachment 8933368 [details]
Bug 1408227 - part 2: WSRunObject::InsertBreak() should take |const EditorRawDOMPoint&| as an argument

https://reviewboard.mozilla.org/r/201916/#review210318

Comment 30

[Makoto Kato [:m_kato]]

Comment on attachment 8933369 [details]
Bug 1408227 - part 3: Redesign WSRunObject::FindRun() with EditorRawDOMPoint

https://reviewboard.mozilla.org/r/201918/#review210320

Comment 31

[Makoto Kato [:m_kato]]

Comment on attachment 8933370 [details]
Bug 1408227 - part 4: WSRunObject::DeleteChars() should take two |const EditorRawDOMPoint&| arguments to specify a range to remove

https://reviewboard.mozilla.org/r/201920/#review210322

Comment 32

[Makoto Kato [:m_kato]]

Comment on attachment 8933371 [details]
Bug 1408227 - part 5: Redesign GetCharAfter(), GetCharBefore(), GetWSPointAfter() and GetWSPointBefore() of WSRunObject

https://reviewboard.mozilla.org/r/201922/#review210326

Comment 33

[Makoto Kato [:m_kato]]

Comment on attachment 8933372 [details]
Bug 1408227 - part 6: WSRunObject::CheckTrailingNBSP() should take |const EditorRawDOMPoint&| instead of a set of container node and offset in it

https://reviewboard.mozilla.org/r/201924/#review210328

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/13d9a39c2f97
part 1: TextEditor::CreateBRImpl() should take |const EditorRawDOMPoint&| for insertion point of new <br> element r=m_kato
https://hg.mozilla.org/integration/autoland/rev/6fc7cf0d7a86
part 2: WSRunObject::InsertBreak() should take |const EditorRawDOMPoint&| as an argument r=m_kato
https://hg.mozilla.org/integration/autoland/rev/33d2e22dc648
part 3: Redesign WSRunObject::FindRun() with EditorRawDOMPoint r=m_kato
https://hg.mozilla.org/integration/autoland/rev/baed101e364b
part 4: WSRunObject::DeleteChars() should take two |const EditorRawDOMPoint&| arguments to specify a range to remove r=m_kato
https://hg.mozilla.org/integration/autoland/rev/ed55a1cbf4ab
part 5: Redesign GetCharAfter(), GetCharBefore(), GetWSPointAfter() and GetWSPointBefore() of WSRunObject r=m_kato
https://hg.mozilla.org/integration/autoland/rev/b4af5fae3d9b
part 6: WSRunObject::CheckTrailingNBSP() should take |const EditorRawDOMPoint&| instead of a set of container node and offset in it r=m_kato

Comment 35

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/13d9a39c2f97
https://hg.mozilla.org/mozilla-central/rev/6fc7cf0d7a86
https://hg.mozilla.org/mozilla-central/rev/33d2e22dc648
https://hg.mozilla.org/mozilla-central/rev/baed101e364b
https://hg.mozilla.org/mozilla-central/rev/ed55a1cbf4ab
https://hg.mozilla.org/mozilla-central/rev/b4af5fae3d9b