dcf51aa102e9e596604c5ea1ad5d44a959ed8729: Bug 1415800 - part 5: Redesign HTMLEditRules::FindNearSelectableNode() r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 10 Nov 2017 01:35:10 +0900 - rev 696009
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415800 - part 5: Redesign HTMLEditRules::FindNearSelectableNode() r?m_kato First, the method name is not correct. It tries to find an editable node near the given DOM point. Therefore, it should be FindNearEditableNode(). Next, the implementation did something odd. E.g., in the first |if| block, when |nearNode| is nullptr, it returns nullptr. However, following |if| block does something only when |nearNode| is nullptr. So, we can get rid of the second |if| block. Then, nobody will change aDirection. So, we can make it not a reference now. Similarly, in |while| block, if |nearNode| becomes nullptr, it returns error. However, following block checks if |nearNode| is NOT nullptr. So, we can get rid of this |if| statement and outdent its block. Additionally, |curNode| isn't necessary. It only increments the refcount redundantly. So, we can get rid of it. Finally, FindNearEditableNode() can return found node directly instead of error code because error code doesn't make sense. Not found an editable node is not illegal. And also it can take EditorRawDOMPoint instead of a set of container, child and offset of the child in the container. MozReview-Commit-ID: CTI581PhJMd
4f10b9a1d41a89f6d82edde16faa951e2432b838: Bug 1415800 - part 4: Redesign HTMLEditor::GetNextHTMLNode() same as similar to EditorBase::GetNext*() r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 10 Nov 2017 00:12:22 +0900 - rev 696008
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415800 - part 4: Redesign HTMLEditor::GetNextHTMLNode() same as similar to EditorBase::GetNext*() r?m_kato HTMLEditor::GetNextHTMLNode() should be redesigned as HTMLEditor::GetNextEditableHTMLNode(nsINode&), HTMLEditor::GetNextEditableHTMLNodeInBlock(nsINode&), HTMLEditor::GetNextEditableHTMLNode(const EditorRawDOMPoint&) and HTMLEditor::GetNextEditableHTMLNodeInBlock(const EditorRawDOMPoint&). Same as GetPreviousEditableHTMLNode*(), we don't need the methods to find non-editable nodes too. MozReview-Commit-ID: JjZauCMblp4
5d078405c46b553ed876cb6d78d2e0e65c882f4d: Bug 1415800 - part 3: Redesign HTMLEditor::GetPriorHTMLNode() as similar to EditorBase::GetPrevious*() r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 09 Nov 2017 23:31:59 +0900 - rev 696007
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415800 - part 3: Redesign HTMLEditor::GetPriorHTMLNode() as similar to EditorBase::GetPrevious*() r?m_kato HTMLEditor::GetPriorHTMLNode() methods are similar to EditorBase::GetPriorNode() which was redesigned with the previous patch. So, it should be redesigned as HTMLEditor::GetPreviousEditableHTMLNode(nsINode&), HTMLEditor::GetPreviousEditableHTMLNode(const EditorRawDOMPoint&), HTMLEditor::GetPreviousEditableHTMLNodeInBlock(nsINode&) and HTMLEditor::GetPreviousEditableHTMLNodeInBlock(const EditorRawDOMPoint&). Note that HTMLEditor::GetPriorHTMLNode() are always return editable node. So, we don't need to create non-editable node methods for them. Although, I don't like the word "HTMLNode" because this can return SVG element or something too. The additional feature of those methods is just checking given node is in active editing host. So, they are for HTML editor, but not returning only HTML nodes. However, I have no better idea with shorter name. MozReview-Commit-ID: 3J4IaBOFjzj
1d08e03f00a0a765b589a60e44a32c1d035cec8d: Bug 1415800 - part 2: Redesign EditorBase::GetNextNode() with EditorRawDOMPoint r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 09 Nov 2017 17:57:00 +0900 - rev 696006
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415800 - part 2: Redesign EditorBase::GetNextNode() with EditorRawDOMPoint r?m_kato An overload of EditorBase::GetNextNode() takes a set of container, child node and offset of the child in the container. Replacing it with EditorRawDOMPoint makes the caller simpler. Additionally, it has two bool arguments, one is for searching if editable node, the other is for searching if in same block. So, they can be hidden with some human readable inline methods. When I was creating this patch, I realized that GetNextNodeInternal(const EditorRawDOMPoint& aPoint) may return aPoint.GetChildAtOffset(). I.e., it starts to search from the specified point rather than next node. On the other hand, GetNextNodeInternal(nsINode& aNode) never returns aNode itself. So, it there is better name instead of "Next", we should take it. But I have no better idea. So, this patch just explains the difference with comments in EditorBase.h. MozReview-Commit-ID: 4Lb6o9SJuhy
52e7826c7efec6b61c9ff8264f47942253546983: Bug 1415800 - part 1: Redesign EditorBase::GetPriorNode() with EditorRawDOMPoint r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 09 Nov 2017 17:08:10 +0900 - rev 696005
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415800 - part 1: Redesign EditorBase::GetPriorNode() with EditorRawDOMPoint r?m_kato An overload of EditorBase::GetPriorNode() takes a set of container, child node and offset of the child in the container. Replacing it with EditorRawDOMPoint makes the caller simpler. Additionally, it has two bool arguments, one is for searching if editable node, the other is for searching if in same block. So, they can be hidden with some human readable inline methods. Finally, "Prior" isn't a term of DOM. So, let's use "Previous" instead. MozReview-Commit-ID: A9uKzHaikY9
a1285db2a916449ded6c517af63d7cda6bdde71f: Bug 1415409 - Make == operator of RangeBoundaryBase compare mRef and mOffset more carefully r?catalinb draft
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 08 Nov 2017 13:35:00 +0900 - rev 696004
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415409 - Make == operator of RangeBoundaryBase compare mRef and mOffset more carefully r?catalinb Currently, RangeBoundaryBase can store either only mRef or mOffset. However, == operator of RangeBoudaryBase still compares mRef simply. However, if one has only mRef and the other has only mOffset, it returns false. This patch makes == operator checks if both mOffset have been set. If so, this checks both mOffset.value() and mRef are matched. However, if mRef of only one of them is nullptr, this doesn't make it check mRef because computing mRef needs some cost and initializing mRef from the other fixes the referring child stronger. If the user of the operator sets only mOffset and wait DOM tree changes, computing mRef may break such callers. If one has only mRef and the other has only mOffset, this patch makes it compute mRef. This is not the best behavior, perhaps. However, there is no way to compare these instances. If this becomes a problem, we should make it create temporary instance, but it'll waste runtime cost. So, let's avoid using this approach for now. Finally, making it check both mRef simply. MozReview-Commit-ID: 4nsW5SYDTiZ
b924f39993f44352a44818873b63197a77a82cfb: Bug 1415509 - part 3: WSRunObject::InsertText() should take |const EditorRawDOMPoint&| as input argument and |EditorRawDOMPoint*| as out argument instead of a set of container, child and offset of the child in the container as in/out argument r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 09 Nov 2017 13:24:06 +0900 - rev 696003
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415509 - part 3: WSRunObject::InsertText() should take |const EditorRawDOMPoint&| as input argument and |EditorRawDOMPoint*| as out argument instead of a set of container, child and offset of the child in the container as in/out argument r?m_kato Like EditorBase::InsertTextImpl(), WSRunObject::InsertText() is really messy. So, it should take same arguments as EditorBase::InsertTextImpl(). MozReview-Commit-ID: 5uKGaxKheRv
fae74aa6a1ae7acc808361033619e73d6425eb0f: Bug 1415509 - part 2: EditorBase::InsertTextImpl() should take |const EditorRawDOMPoint&| argument as input and |EditorRawDOMPoint*| as out argument instead of a set of container, child and offset of the child in the container as in/out argument r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 09 Nov 2017 01:00:36 +0900 - rev 696002
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415509 - part 2: EditorBase::InsertTextImpl() should take |const EditorRawDOMPoint&| argument as input and |EditorRawDOMPoint*| as out argument instead of a set of container, child and offset of the child in the container as in/out argument r?m_kato EditorBase::InsertTextImpl() takes |nsCOMPtr<nsINode>*|, |nsCOMPtr<nsIContent>*| and |int32_t| as in/out arguments for container, child and offset of the child in the container. But this makes the callers really hard to read and ugly. So, we should make the method take |const EditorRawDOMPoint&| argument as input and |EditorRawDOMPoint*| as out argument. MozReview-Commit-ID: 2ijIfGl4Zo7
275ce0d3ab878ae7965d4a32cf48617bdb313478: Bug 1415509 - part 1: EditorBase::FindBetterInsertionPoint() should take an EditorRawDOMPoint argument for input and return EditorRawDOMPoint for the result r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 08 Nov 2017 21:55:10 +0900 - rev 696001
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415509 - part 1: EditorBase::FindBetterInsertionPoint() should take an EditorRawDOMPoint argument for input and return EditorRawDOMPoint for the result r?m_kato EditorBase::FindBetterInsertionPoint() now use 3 in/out arguments. This is really ugly and making the callers hard to read. So, let's make it take an argument whose type is |const EditorRawDOMPoint&| and return other EditorRawDOMPoint instance. Additionally, this fixes bugs of text node length checks in the method. Basically, this shouldn't affect to any actual behavior, though. That is because text node shouldn't be able to have string longer than INT32_MAX. MozReview-Commit-ID: FClUQSJzd8c
e7763e06535d6820e16b9ec4a29f270f59ccc80a: Bug 1415231 - Add crash tests for a regression of bug 1406482 which has been fixed by bug 1415509 r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 10 Nov 2017 10:58:33 +0900 - rev 696000
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415231 - Add crash tests for a regression of bug 1406482 which has been fixed by bug 1415509 r?m_kato This crash test can cause crash before landing the patches for bug 1415509. So, let's take this for regression test. MozReview-Commit-ID: 652wi49e720
1500a75dad10f3890db7f33aeeec66e202d35599: Bug 1415445 - part 4: EditorBase::CreateNode() should take EditorRawDOMPoint as insertion point instead of a set of container, child and offset of the child in the container r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 08 Nov 2017 20:23:10 +0900 - rev 695999
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415445 - part 4: EditorBase::CreateNode() should take EditorRawDOMPoint as insertion point instead of a set of container, child and offset of the child in the container r?m_kato EditorBase::CreateNode() should take EditorRawDOMPoint as insertion point of the new element instead of a set of container, child and offset of the child in the container. This patch initializes EditorRawDOMPoint with original 3 arguments as far as possible. If the relation of them are broken, MOZ_ASSERT in RawRangeBoundary constructor detects existing bugs. MozReview-Commit-ID: 2N55S6pRv7k
bc13f1b018a963f2df2fd2904ba797d5ad3d2fc5: Bug 1415445 - part 3: nsIEditActionListener's WillCreateElement() and DidCreateElement() should take child node at insertion point or new node itself rather than the container node and offset in it r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 08 Nov 2017 18:16:46 +0900 - rev 695998
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415445 - part 3: nsIEditActionListener's WillCreateElement() and DidCreateElement() should take child node at insertion point or new node itself rather than the container node and offset in it r?m_kato nsIEditActionListener::WillCreateElement() and nsIEditActionListener::DidCreateElement() are implemented only by m-c. So, we can remove a set of container node and offset in it from their argument. Instead, WillCreateElement() should take a node which will be next sibling of the new node. Note that only implementation of them is, HTMLEditRules::DidCreateElement(). So, we can get rid of them and can call HTMLEditRules::DidCreateElement() directly from EditorBase::CreateNode(). However, such change should be done in another bug which checks all nsIEditActionListener method implementations. MozReview-Commit-ID: 4LQEs2WwrVC
aa4fb182310a044fb8bd35a709b730ca5860ef0d: Bug 1415445 - part 2: EditorBase::CreateTxnForCreateElement() should take EditorRawDOMPoint for insertion point r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 08 Nov 2017 17:23:28 +0900 - rev 695997
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415445 - part 2: EditorBase::CreateTxnForCreateElement() should take EditorRawDOMPoint for insertion point r?m_kato The constructor of CreateElementTransaction now takes EditorRawDOMPoint instead of a set of container node and offset in it. So, its only user, EditorBase::CreateTxnForCreateElement(), should take EditorRawDOMPoint too. MozReview-Commit-ID: A8QfPM3LRii
4d5f53f51f67336193ebc419bf9d6cb8d8861c81: Bug 1415445 - part 1: CreateElementTransaction should use EditorRawDOMPoint and RangeBoundary r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 08 Nov 2017 16:49:06 +0900 - rev 695996
Push 88610 by masayuki@d-toybox.com at Fri, 10 Nov 2017 04:02:33 +0000
Bug 1415445 - part 1: CreateElementTransaction should use EditorRawDOMPoint and RangeBoundary r?m_kato The constructor of CreateElementTransaction should use EditorRawDOMPoint to receive insertion point of the new element. Then, it should be stored with RangeBoundary. With this change, CreateElementTransaction doesn't need to compute child node at insertion point. Additionally, this creates InsertNode() method. Current code works differently when DoTransaction() is called and RedoTransaction() is called. MozReview-Commit-ID: 8ujhmzn65Wg
03673328a7ff88abab3d3712bcd8efab5b9e090b: Bug 1416094 - WIP backup draft
DimiL <dlee@mozilla.com> - Thu, 09 Nov 2017 16:12:03 +0800 - rev 695995
Push 88609 by bmo:dlee@mozilla.com at Fri, 10 Nov 2017 03:58:43 +0000
Bug 1416094 - WIP backup MozReview-Commit-ID: IHZW0Zkfnt5
c62707f67d8baec7f87ac473980aaa8b781255c6: Bug 654787 - part8: Fire seeking and seeked events when looping back to the beginning; r?jwwang draft
Chun-Min Chang <chun.m.chang@gmail.com> - Wed, 08 Nov 2017 17:17:12 +0800 - rev 695994
Push 88608 by bmo:cchang@mozilla.com at Fri, 10 Nov 2017 03:55:46 +0000
Bug 654787 - part8: Fire seeking and seeked events when looping back to the beginning; r?jwwang MozReview-Commit-ID: GJlus3NhUQS
e041096f0f9a368fb652ade56da8fb2d98d3b195: Bug 654787 - part7: Stop playing and decoding when looping is cancelled; r?jwwang draft
Chun-Min Chang <chun.m.chang@gmail.com> - Wed, 08 Nov 2017 17:05:28 +0800 - rev 695993
Push 88608 by bmo:cchang@mozilla.com at Fri, 10 Nov 2017 03:55:46 +0000
Bug 654787 - part7: Stop playing and decoding when looping is cancelled; r?jwwang MozReview-Commit-ID: FM3ZQvyZC2a
8fedaccd543dddbe4c85c40526ba32fae165ad9c: Bug 654787 - part6: Correct the playback position while looping; r?jwwang draft
Chun-Min Chang <chun.m.chang@gmail.com> - Fri, 10 Nov 2017 10:56:07 +0800 - rev 695992
Push 88608 by bmo:cchang@mozilla.com at Fri, 10 Nov 2017 03:55:46 +0000
Bug 654787 - part6: Correct the playback position while looping; r?jwwang MozReview-Commit-ID: KCwFpepdget
941fff17e0f36963961ba1253d850f8616a5d1ec: Bug 654787 - part5: Add the looping-offset time to audio data; r=jwwang draft
Chun-Min Chang <chun.m.chang@gmail.com> - Fri, 10 Nov 2017 11:39:03 +0800 - rev 695991
Push 88608 by bmo:cchang@mozilla.com at Fri, 10 Nov 2017 03:55:46 +0000
Bug 654787 - part5: Add the looping-offset time to audio data; r=jwwang MozReview-Commit-ID: 5qMw09YjyUC
14bfd0a50faf9473efe5d1a04adeecbad999652a: Bug 654787 - part4: Keep decoding to MDSM in ReaderProxy when looping is on; r?jwwang draft
Chun-Min Chang <chun.m.chang@gmail.com> - Fri, 10 Nov 2017 11:40:24 +0800 - rev 695990
Push 88608 by bmo:cchang@mozilla.com at Fri, 10 Nov 2017 03:55:46 +0000
Bug 654787 - part4: Keep decoding to MDSM in ReaderProxy when looping is on; r?jwwang MozReview-Commit-ID: 8Bf9nD760Jk
(0) -300000 -100000 -30000 -10000 -3000 -1000 -300 -100 -50 -20 +20 +50 +100 +300 +1000 +3000 +10000 +30000 +100000 tip