searching for reviewer(m_kato)
3d27e4213ccc750e392a6b5717247886983e8d5a: Bug 1484127 - part 1: Create HTMLEditor::GetTableCellElementAt() for internal use of nsITableEditor::GetCellAt() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 23 Aug 2018 06:39:30 +0000 - rev 831000
Push 118868 by bmo:zjz@zjz.name at Fri, 24 Aug 2018 07:04:39 +0000
Bug 1484127 - part 1: Create HTMLEditor::GetTableCellElementAt() for internal use of nsITableEditor::GetCellAt() r=m_kato nsITableEditor::GetCellAt() is an XPCOM method, but this is called internally a lot. So, we should create non-virtual method for internal use. The XPCOM method retrieves a <table> element if given element is nullptr. However, no internal user needs this feature. So, we can create GetTableCellElementAt() simply. Then, we can get rid of nsresult and ErrorResult from it. Differential Revision: https://phabricator.services.mozilla.com/D3956
b95c3522fa1e72dcd5f50726a08477521ab1e951: Bug 1484127 - part 0: Add automated tests for nsITableEditor::GetCellAt() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 23 Aug 2018 06:13:22 +0000 - rev 830999
Push 118868 by bmo:zjz@zjz.name at Fri, 24 Aug 2018 07:04:39 +0000
Bug 1484127 - part 0: Add automated tests for nsITableEditor::GetCellAt() r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D3955
3cf8d56c3f33785c07812a41e172a10128878fb3: Bug 1484125 - part 1: Create TableSize struct to compute and store number of rows and columns of a <table> element r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 23 Aug 2018 07:32:16 +0000 - rev 830998
Push 118868 by bmo:zjz@zjz.name at Fri, 24 Aug 2018 07:04:39 +0000
Bug 1484125 - part 1: Create TableSize struct to compute and store number of rows and columns of a <table> element r=m_kato HTMLEditor::GetTableSize() is an XPCOM method but used internally a lot. Therefore, it shouldn't be called for internal use. Additionally, the callers need to declare two int32_t variables, but this causes the code messy. Therefore, this patch creates TableSize struct and it implements HTMLEditor::GetTableSize(). Then, all callers of it is replaced with TableSize struct. New TableSize struct does not support computes <table> element from anchor of Selection since there is no user of this in C++ code. Differential Revision: https://phabricator.services.mozilla.com/D3953
5c0edfef06dd997312d977fbe69fa6feceb4a43b: Bug 1484125 - part 0: Add automated tests for nsITableEditor::GetTableSize() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 23 Aug 2018 06:42:11 +0000 - rev 830997
Push 118868 by bmo:zjz@zjz.name at Fri, 24 Aug 2018 07:04:39 +0000
Bug 1484125 - part 0: Add automated tests for nsITableEditor::GetTableSize() r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D3951
2dcfee19716dae21e9d8ce3c1a00b7db930815b3: Bug 1484124 - part 1: Create HTMLEditor::GetCellIndexes() class to get and store indexes of a table cell r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 22 Aug 2018 03:34:40 +0000 - rev 830759
Push 118854 by bmo:a.beingessner@gmail.com at Wed, 22 Aug 2018 20:13:35 +0000
Bug 1484124 - part 1: Create HTMLEditor::GetCellIndexes() class to get and store indexes of a table cell r=m_kato HTMLEditor::GetCellIndexes() is an XPCOM method and used a lot internally. So, we need alternative way to retrieve indexes of a cell without virtual calls. In a lot of places, receiving indexes with 2 int32_t variables causes the code messy and that causes making it harder to understand which are index for same cell and where they come from. So, making both of them stored one variable makes the callers simpler. Therefore, this patch creates CellIndexes stack class to get and store the result simply. Then, this makes all callers of GetCellIndexes() use this new class and makes GetCellIndexes() also use this new class. FYI: This patch does NOT put ErrorResult instances in small block scope as far as possible. The reason is, I see its destructor in profile sometimes. I don't think that we should use nsresult& instead of ErrorResult& only for this performance reason, but I think that creating each instance in loops does not make sense. Differential Revision: https://phabricator.services.mozilla.com/D3849
325f0f4b0f11616b8b7268521a2b6e58715684fd: Bug 1484124 - part 0: Add automated tests for nsITableEditor::GetCellIndexes() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 22 Aug 2018 03:35:05 +0000 - rev 830758
Push 118854 by bmo:a.beingessner@gmail.com at Wed, 22 Aug 2018 20:13:35 +0000
Bug 1484124 - part 0: Add automated tests for nsITableEditor::GetCellIndexes() r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D3848
357d549504fb6e1330bc0aa152cafcd27b0c9a2a: Bug 1484115 - part 2: Get rid of nsITableEditor.getNextRow() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 22 Aug 2018 06:52:16 +0000 - rev 830751
Push 118854 by bmo:a.beingessner@gmail.com at Wed, 22 Aug 2018 20:13:35 +0000
Bug 1484115 - part 2: Get rid of nsITableEditor.getNextRow() r=m_kato Nobody uses nsITableEditor.getNextRow(). Therefore, this patch removes this XPCOM API. Differential Revision: https://phabricator.services.mozilla.com/D3949
05f438b601b5e6d4df5dfef992c2cf1d153757ea: Bug 1484115 - part 1: Create HTMLEditor::GetNextTableRowElement() for internal use of nsITableEditor::GetNextRow() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 22 Aug 2018 06:52:07 +0000 - rev 830750
Push 118854 by bmo:a.beingessner@gmail.com at Wed, 22 Aug 2018 20:13:35 +0000
Bug 1484115 - part 1: Create HTMLEditor::GetNextTableRowElement() for internal use of nsITableEditor::GetNextRow() r=m_kato nsITableEditor::GetNextRow() is an XPCOM method. Therefore, we should have a non-virtual method for internal use of it. This changes the definition in nsITableEditor. First, it allows only <tr> element as what HTMLEditor::GetNextRow() has actually done. Then, changes the return type to Element since it always returns an element node. Differential Revision: https://phabricator.services.mozilla.com/D3948
2dd1517113c3b4b77ed0baf5313e9b0ae1a6a873: Bug 1484693 - Fix some nits of test_resizers_resizing_elements.html r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 22 Aug 2018 06:44:51 +0000 - rev 830748
Push 118854 by bmo:a.beingessner@gmail.com at Wed, 22 Aug 2018 20:13:35 +0000
Bug 1484693 - Fix some nits of test_resizers_resizing_elements.html r=m_kato There are 2 bugs: One is a simple mistake. kTest is each item of the tests, kTests is array of all tests. When it needs to refer kTest.isAbsolutePosition, it referred kTests.isAbsolutePosiiton. Therefore, the test always failed to enable editing UI for absolute positioned element. The other is, this test requires to disable inline-table-editing UI (which is add or remove rows and columns). Note that even if the UI is disabled, resizers is available for <table> elements. Differential Revision: https://phabricator.services.mozilla.com/D3954
090edc3b798b91e876b40da371c515397743f5fd: Bug 1484113 - part 1: Create HTMLEditor::GetFirstTableRowElement() for internal use of nsITableEditor::GetFirstRow() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 22 Aug 2018 01:20:23 +0000 - rev 830731
Push 118854 by bmo:a.beingessner@gmail.com at Wed, 22 Aug 2018 20:13:35 +0000
Bug 1484113 - part 1: Create HTMLEditor::GetFirstTableRowElement() for internal use of nsITableEditor::GetFirstRow() r=m_kato nsITableEditor::GetFirstRow() is an XPCOM method, so, for internal use, we should create non-virtual method, that is GetFirstTableRowElement(). This patch makes it never return NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND since nobody refers it and it's detectable. If the method returns nullptr without error, it's the case of NS_SUCCESS_EDITOR_ELEMENT_NOT_FOUND. Additionally, this patch changes the return type of GetFirstRow() from Node to Element since it always return an Element node if not null. Differential Revision: https://phabricator.services.mozilla.com/D3780
af33be3aee822ddf186798da08ca3d9863557b30: Bug 1484113 - part 0: Create automated tests for nsITableEditor::GetFirstRow() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 22 Aug 2018 02:16:36 +0000 - rev 830730
Push 118854 by bmo:a.beingessner@gmail.com at Wed, 22 Aug 2018 20:13:35 +0000
Bug 1484113 - part 0: Create automated tests for nsITableEditor::GetFirstRow() r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D3779
4f0a12bcb4017a46280b133aa2be83ca49b6d14a: Bug 1484110 - part 3: HTMLEditor::RefereshEditingUI() should refresh UIs when one of them is changed to enabled or disabled r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 17 Aug 2018 19:03:02 +0900 - rev 830451
Push 118832 by bmo:ntim.bugs@gmail.com at Tue, 21 Aug 2018 13:33:17 +0000
Bug 1484110 - part 3: HTMLEditor::RefereshEditingUI() should refresh UIs when one of them is changed to enabled or disabled r=m_kato HTMLEditor::RefereshEditingUI() works only with enabled UIs. Therefore, if UI is disabled while it's visible, it keeps shown. This is too bad if web apps tries to disable the Gecko specific UIs after we show some of them. This patch adds HTMLEditor::HideAnonymousEditingUIsIfUnnecessary() to hide unnecessary UIs and makes RefereshEditingUI() call it always.
6a6a79e2c19b5b08ffcef13fc0741f98ddcecf19: Bug 1484110 - part 2: Rewrite HTMLEditor::HideInlineTableEditingUI() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 17 Aug 2018 18:23:13 +0900 - rev 830450
Push 118832 by bmo:ntim.bugs@gmail.com at Tue, 21 Aug 2018 13:33:17 +0000
Bug 1484110 - part 2: Rewrite HTMLEditor::HideInlineTableEditingUI() r=m_kato First, HTMLEditor::HideInlineTableEditingUI() always returns NS_OK. So, we can change its return type to void. Additionally, it removes each UI from the DOM tree one by one. However, each mutation could cause showing same UI again. In such case, ShowInlineTableEditingUI() overwrites each UI with newly created element. Then, HTMLEditor cannot remove the old UI anymore. Therefore, this patch moves all members of the UI into local variables first.
08d0e0c32a53307e3f75645206000244375c4314: Bug 1484110 - part 1: Create HTMLEditor::RefereshEditingUI() for internal use of nsIHTMLEditor::CheckSelectionStateForAnonymousButtons() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 17 Aug 2018 17:56:28 +0900 - rev 830449
Push 118832 by bmo:ntim.bugs@gmail.com at Tue, 21 Aug 2018 13:33:17 +0000
Bug 1484110 - part 1: Create HTMLEditor::RefereshEditingUI() for internal use of nsIHTMLEditor::CheckSelectionStateForAnonymousButtons() r=m_kato HTMLEditor::CheckSelectionStateForAnonymousButtons() is called a lot internally. Especially, its virtual call cost may make damage to our performance since it's called from a selection listener. So, we should create non-virtual method, RefereshEditingUI() for internal use.
dcdd7d0adcc07d883c971b11e1d33a0212f63327: Bug 1484094 - Don't allow the caret to be placed within a ligated emoji sequence. r=m_kato
Jonathan Kew <jkew@mozilla.com> - Mon, 20 Aug 2018 09:07:40 +0100 - rev 830237
Push 118825 by bmo:rob@robwu.nl at Mon, 20 Aug 2018 17:24:40 +0000
Bug 1484094 - Don't allow the caret to be placed within a ligated emoji sequence. r=m_kato
541fbb29f21d262d88d0ae08776606db0b62d8f6: Bug 1449564 - part 4: Make users can show Gecko specific editing UIs with new prefs r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 16 Aug 2018 13:51:36 +0900 - rev 830207
Push 118824 by bmo:hskupin@gmail.com at Mon, 20 Aug 2018 15:49:57 +0000
Bug 1449564 - part 4: Make users can show Gecko specific editing UIs with new prefs r=m_kato Even after we disable Gecko specific editing UIs by default, web apps can enable them with execCommand. However, until such web apps change their behavior, users cannot use Gecko specific UIs. At least for now, we should make users can enable them by default. MozReview-Commit-ID: AuAdw4FQ4He
e93a23a4741c7760d76e33c3515e9b6ee2f5e26d: Bug 1449564 - part 3: Make absolute position editor listen to mouse events at the system event group r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 05 Apr 2018 00:32:32 +0900 - rev 830206
Push 118824 by bmo:hskupin@gmail.com at Mon, 20 Aug 2018 15:49:57 +0000
Bug 1449564 - part 3: Make absolute position editor listen to mouse events at the system event group r=m_kato Currently, absolute position editor listens to mouse events at the default event group to handle dragging of positioner. However, this is blocked by a call of Event.stopPropagation() in web apps unexpectedly. Therefore, we should make it listen to the events at the system event group instead. MozReview-Commit-ID: Hoa8c9QvMuG
ccb713187e45a0ce52cf0ebe8969ddac9608e215: Bug 1449564 - part 2: Make absolute positioned element editor disabled in default and make it possible to enable it with new command r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 04 Apr 2018 22:27:49 +0900 - rev 830205
Push 118824 by bmo:hskupin@gmail.com at Mon, 20 Aug 2018 15:49:57 +0000
Bug 1449564 - part 2: Make absolute positioned element editor disabled in default and make it possible to enable it with new command r=m_kato We have another built-in UI of editor which is not implemented by any other browsers. That is a draggable handler to move absolute positioned elements. So, we should disable it in default for compatibility with the other browsers. However, different from resizers and inline table editor, we don't have command to enable/disable this feature but for backward compatibility, we should have it. Therefore, this patch adds new command "enableAbsolutePositionEditor". Note that whether resizing UI is available only with enableObjectResizing state is different from enableInlineTableEditing command. Resizers for absolute positioned elements are NOT available both enableObjectResizing and enableAbsolutePositionEditor are enabled. Additionally, this adds automated tests to check basic functions of absolute positioned editor. MozReview-Commit-ID: 9ZSGB8tLpFw
4e0960a6ffb6c283f29d75ba5bc2585df4c3caf0: Bug 1449564 - part 1: Disable object resizer and inline table editor in default r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 02 Apr 2018 17:26:46 +0900 - rev 830204
Push 118824 by bmo:hskupin@gmail.com at Mon, 20 Aug 2018 15:49:57 +0000
Bug 1449564 - part 1: Disable object resizer and inline table editor in default r=m_kato Gecko supports resizers of <img> elements and <table>, <td>, <th> elements and has UI to remove existing table row or column in default. However, the other browsers don't have such UI and web apps need to disable this feature with calling both: document.execCommand("enableObjectResizing", false, false); document.execCommand("enableInlineTableEditing", false, false); for avoiding conflicting with their own features to edit such elements. Therefore, it doesn't make sense to keep enabling them in default only on Gecko. If web apps want to keep using these features, they should call: document.execCommand("enableObjectResizing", false, true); document.execCommand("enableInlineTableEditing", false, true); at initializing the editor. And also this patch fixes bugs of document.queryCommandState("enableObjectResizing") and document.queryCommandState("enableInlineTableEditing"). They always return false even after calling document.execCommand(..., false, true) since nsSetDocumentStateCommand::GetCommandStateParams() sets bool value as STATE_ATTRIBUTE. However, nsHTMLDocument::QueryCommandValue() which is the caller referring STATE_ATTRIBUTE doesn't treat it as bool value. And also those commands are related to state of document. Therefore, they should be return as bool value of STATE_ALL instead. Then, nsHTMLDocument::QueryCommandState() returns the state as expected. Note that those commands are supported only by Gecko. So, we don't need to worry about the compatibility. Finally, this patch rewrites 2 existing tests to check basic behavior of resizers and appearance of resizers. Note that this patch does not add new tests to test inline table editor since it's difficult to test the behavior with current API. Perhaps, we should add an API to nsIHTMLEditor to retrieve each anonymous elements in another bug since it requires to add wrapping API of SpecialPowers. MozReview-Commit-ID: 1FhYo5vcV60
4b5906a29b0cd71fbb9bbacf8640b00892878ab2: Bug 1484092 - part 3: IsLinkTag() and IsNamedAnchorTag() should compare with nsGkAtoms r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 17 Aug 2018 14:51:40 +0000 - rev 830039
Push 118809 by bmo:ntim.bugs@gmail.com at Sat, 18 Aug 2018 10:35:28 +0000
Bug 1484092 - part 3: IsLinkTag() and IsNamedAnchorTag() should compare with nsGkAtoms r=m_kato The methods compared with const characters since we've supported "namedanchor" which is not in nsGkAtoms. Now, it's dropped so that we can compare given atom with nsGkAtoms. Differential Revision: https://phabricator.services.mozilla.com/D3586
a2a95c8855db46baa7009bdc25f1555bd5f1f1e4: Bug 1484092 - part 2: Drop supporting "namedanchor" special element name from nsIHTMLEditor::GetSelectedElement(), nsIHTMLEditor::GetElementOrParentByTagName() and nsIHTMLEditor::CreateElementWithDefaults() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 17 Aug 2018 14:50:56 +0000 - rev 830038
Push 118809 by bmo:ntim.bugs@gmail.com at Sat, 18 Aug 2018 10:35:28 +0000
Bug 1484092 - part 2: Drop supporting "namedanchor" special element name from nsIHTMLEditor::GetSelectedElement(), nsIHTMLEditor::GetElementOrParentByTagName() and nsIHTMLEditor::CreateElementWithDefaults() r=m_kato Nobody (including comm-central and BlueGriffon) does not use "namedanchor" special element name with those XPCOMs. Of course, our internal callers too. Therefore, we can drop. Note that there is no static Atom for this, so, keeping it makes unnecessary runtime cost for Firefox users. This could cause breaking some legacy add-ons for Thunderbird. However, they can use "anchor" special element name for same purpose. Differential Revision: https://phabricator.services.mozilla.com/D3585
717fb8632016211ce9cf8a7267be60e52f7e77c8: Bug 1484092 - part 1: Make HTMLEditor::GetElementOrParentByTagName() use nsAtom for the tag name r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 17 Aug 2018 14:06:18 +0000 - rev 830035
Push 118809 by bmo:ntim.bugs@gmail.com at Sat, 18 Aug 2018 10:35:28 +0000
Bug 1484092 - part 1: Make HTMLEditor::GetElementOrParentByTagName() use nsAtom for the tag name r=m_kato HTMLElementOrParentByTagName() is the last user of IsLinkTag(const nsAString&) and IsNamedAnchorTag(const nsAString&). For making their maintenance easier, let's make GetElementOrParentByTagName() take const nsAtom& for tag name. GetElementOrParentByTagName() has two functions, one is looking for an element starting from a node. The other is, if the start node is nullptr, it retrieves anchor node of Selection as start node. Therefore, this patch splits the first part to GetElementOrParentByTagNameInternal(). Then, creates its wrapper which retrieves anchor of Selection automatically, GetElementOrParentByTagNameAtSelection(). Additionally, this patch makes all internal callers of HTMLEditor use GetElementOrParentByTagNameInternal() or GetElementOrParentByTagNameAtSelection() directly. Then, public method, GetElementOrParentByTagName() is called only by outer classes. Note that some callers use both GetElementOrParentByTagNameInternal() and GetElementOrParentByTagNameAtSelection() since they don't check whether setting node is nullptr. They may be bug of them. We should investigate the API callers later. Differential Revision: https://phabricator.services.mozilla.com/D3584
10fdd041f1b5ef4ab557d6d6d7c4b405ae55764b: Bug 1484092 - part 2: Drop supporting "namedanchor" special element name from nsIHTMLEditor::GetSelectedElement(), nsIHTMLEditor::GetElementOrParentByTagName() and nsIHTMLEditor::CreateElementWithDefaults() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 17 Aug 2018 07:41:55 +0000 - rev 830023
Push 118809 by bmo:ntim.bugs@gmail.com at Sat, 18 Aug 2018 10:35:28 +0000
Bug 1484092 - part 2: Drop supporting "namedanchor" special element name from nsIHTMLEditor::GetSelectedElement(), nsIHTMLEditor::GetElementOrParentByTagName() and nsIHTMLEditor::CreateElementWithDefaults() r=m_kato Nobody (including comm-central and BlueGriffon) does not use "namedanchor" special element name with those XPCOMs. Of course, our internal callers too. Therefore, we can drop. Note that there is no static Atom for this, so, keeping it makes unnecessary runtime cost for Firefox users. This could cause breaking some legacy add-ons for Thunderbird. However, they can use "anchor" special element name for same purpose. Differential Revision: https://phabricator.services.mozilla.com/D3585
d0b14e8711dfbe1063f0e9ea310118d486e46652: Bug 1484092 - part 1: Make HTMLEditor::GetElementOrParentByTagName() use nsAtom for the tag name r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 17 Aug 2018 10:04:42 +0000 - rev 830022
Push 118809 by bmo:ntim.bugs@gmail.com at Sat, 18 Aug 2018 10:35:28 +0000
Bug 1484092 - part 1: Make HTMLEditor::GetElementOrParentByTagName() use nsAtom for the tag name r=m_kato HTMLElementOrParentByTagName() is the last user of IsLinkTag(const nsAString&) and IsNamedAnchorTag(const nsAString&). For making their maintenance easier, let's make GetElementOrParentByTagName() take const nsAtom& for tag name. GetElementOrParentByTagName() has two functions, one is looking for an element starting from a node. The other is, if the start node is nullptr, it retrieves anchor node of Selection as start node. Therefore, this patch splits the first part to GetElementOrParentByTagNameInternal(). Then, creates its wrapper which retrieves anchor of Selection automatically, GetElementOrParentByTagNameAtSelection(). Additionally, this patch makes all internal callers of HTMLEditor use GetElementOrParentByTagNameInternal() or GetElementOrParentByTagNameAtSelection() directly. Then, public method, GetElementOrParentByTagName() is called only by outer classes. Note that some callers use both GetElementOrParentByTagNameInternal() and GetElementOrParentByTagNameAtSelection() since they don't check whether setting node is nullptr. They may be bug of them. We should investigate the API callers later. Differential Revision: https://phabricator.services.mozilla.com/D3584
1f8f46f45d60c42b231e0f6731875dc49048e06b: Bug 1449564 - part 4: Make users can show Gecko specific editing UIs with new prefs r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 16 Aug 2018 13:51:36 +0900 - rev 830012
Push 118808 by masayuki@d-toybox.com at Sat, 18 Aug 2018 08:51:19 +0000
Bug 1449564 - part 4: Make users can show Gecko specific editing UIs with new prefs r?m_kato Even after we disable Gecko specific editing UIs by default, web apps can enable them with execCommand. However, until such web apps change their behavior, users cannot use Gecko specific UIs. At least for now, we should make users can enable them by default. MozReview-Commit-ID: AuAdw4FQ4He
0aa4ba4f041b207e9c400b0ef18ef13519b11c5b: Bug 1449564 - part 3: Make absolute position editor listen to mouse events at the system event group r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 05 Apr 2018 00:32:32 +0900 - rev 830011
Push 118808 by masayuki@d-toybox.com at Sat, 18 Aug 2018 08:51:19 +0000
Bug 1449564 - part 3: Make absolute position editor listen to mouse events at the system event group r?m_kato Currently, absolute position editor listens to mouse events at the default event group to handle dragging of positioner. However, this is blocked by a call of Event.stopPropagation() in web apps unexpectedly. Therefore, we should make it listen to the events at the system event group instead. MozReview-Commit-ID: Hoa8c9QvMuG
d21676503e6642937f54219f0e3c5a36682cd426: Bug 1449564 - part 2: Make absolute positioned element editor disabled in default and make it possible to enable it with new command r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 04 Apr 2018 22:27:49 +0900 - rev 830010
Push 118808 by masayuki@d-toybox.com at Sat, 18 Aug 2018 08:51:19 +0000
Bug 1449564 - part 2: Make absolute positioned element editor disabled in default and make it possible to enable it with new command r?m_kato We have another built-in UI of editor which is not implemented by any other browsers. That is a draggable handler to move absolute positioned elements. So, we should disable it in default for compatibility with the other browsers. However, different from resizers and inline table editor, we don't have command to enable/disable this feature but for backward compatibility, we should have it. Therefore, this patch adds new command "enableAbsolutePositionEditor". Note that whether resizing UI is available only with enableObjectResizing state is different from enableInlineTableEditing command. Resizers for absolute positioned elements are NOT available both enableObjectResizing and enableAbsolutePositionEditor are enabled. Additionally, this adds automated tests to check basic functions of absolute positioned editor. MozReview-Commit-ID: 9ZSGB8tLpFw
641d0c0f8190fc7329a47450c16901621491eb20: Bug 1449564 - part 1: Disable object resizer and inline table editor in default r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 02 Apr 2018 17:26:46 +0900 - rev 830009
Push 118808 by masayuki@d-toybox.com at Sat, 18 Aug 2018 08:51:19 +0000
Bug 1449564 - part 1: Disable object resizer and inline table editor in default r?m_kato Gecko supports resizers of <img> elements and <table>, <td>, <th> elements and has UI to remove existing table row or column in default. However, the other browsers don't have such UI and web apps need to disable this feature with calling both: document.execCommand("enableObjectResizing", false, false); document.execCommand("enableInlineTableEditing", false, false); for avoiding conflicting with their own features to edit such elements. Therefore, it doesn't make sense to keep enabling them in default only on Gecko. If web apps want to keep using these features, they should call: document.execCommand("enableObjectResizing", false, true); document.execCommand("enableInlineTableEditing", false, true); at initializing the editor. And also this patch fixes bugs of document.queryCommandState("enableObjectResizing") and document.queryCommandState("enableInlineTableEditing"). They always return false even after calling document.execCommand(..., false, true) since nsSetDocumentStateCommand::GetCommandStateParams() sets bool value as STATE_ATTRIBUTE. However, nsHTMLDocument::QueryCommandValue() which is the caller referring STATE_ATTRIBUTE doesn't treat it as bool value. And also those commands are related to state of document. Therefore, they should be return as bool value of STATE_ALL instead. Then, nsHTMLDocument::QueryCommandState() returns the state as expected. Note that those commands are supported only by Gecko. So, we don't need to worry about the compatibility. Finally, this patch rewrites 2 existing tests to check basic behavior of resizers and appearance of resizers. Note that this patch does not add new tests to test inline table editor since it's difficult to test the behavior with current API. Perhaps, we should add an API to nsIHTMLEditor to retrieve each anonymous elements in another bug since it requires to add wrapping API of SpecialPowers. MozReview-Commit-ID: 1FhYo5vcV60
0ed4fce8ca6071a2fa61245e39deb1844558a68e: Bug 1484092 - part 3: IsLinkTag() and IsNamedAnchorTag() should compare with nsGkAtoms r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 17 Aug 2018 12:19:27 +0900 - rev 830008
Push 118808 by masayuki@d-toybox.com at Sat, 18 Aug 2018 08:51:19 +0000
Bug 1484092 - part 3: IsLinkTag() and IsNamedAnchorTag() should compare with nsGkAtoms r?m_kato The methods compared with const characters since we've supported "namedanchor" which is not in nsGkAtoms. Now, it's dropped so that we can compare given atom with nsGkAtoms.
40ab8cc3e37da71e07c4371aa1c115866c66f61c: Bug 1484092 - part 2: Drop supporting "namedanchor" special element name from nsIHTMLEditor::GetSelectedElement(), nsIHTMLEditor::GetElementOrParentByTagName() and nsIHTMLEditor::CreateElementWithDefaults() r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 17 Aug 2018 12:00:44 +0900 - rev 830007
Push 118808 by masayuki@d-toybox.com at Sat, 18 Aug 2018 08:51:19 +0000
Bug 1484092 - part 2: Drop supporting "namedanchor" special element name from nsIHTMLEditor::GetSelectedElement(), nsIHTMLEditor::GetElementOrParentByTagName() and nsIHTMLEditor::CreateElementWithDefaults() r?m_kato Nobody (including comm-central and BlueGriffon) does not use "namedanchor" special element name with those XPCOMs. Of course, our internal callers too. Therefore, we can drop. Note that there is no static Atom for this, so, keeping it makes unnecessary runtime cost for Firefox users. This could cause breaking some legacy add-ons for Thunderbird. However, they can use "anchor" special element name for same purpose.
d91d30ba2cc86c32061a898ca1bac07402c918b0: Bug 1484092 - part 1: Make HTMLEditor::GetElementOrParentByTagName() use nsAtom for the tag name r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 17 Aug 2018 11:54:58 +0900 - rev 830006
Push 118808 by masayuki@d-toybox.com at Sat, 18 Aug 2018 08:51:19 +0000
Bug 1484092 - part 1: Make HTMLEditor::GetElementOrParentByTagName() use nsAtom for the tag name r?m_kato HTMLElementOrParentByTagName() is the last user of IsLinkTag(const nsAString&) and IsNamedAnchorTag(const nsAString&). For making their maintenance easier, let's make GetElementOrParentByTagName() take const nsAtom& for tag name. GetElementOrParentByTagName() has two functions, one is looking for an element starting from a node. The other is, if the start node is nullptr, it retrieves anchor node of Selection as start node. Therefore, this patch splits the first part to GetElementOrParentByTagNameInternal(). Then, creates its wrapper which retrieves anchor of Selection automatically, GetElementOrParentByTagNameAtSelection(). Additionally, this patch makes all internal callers of HTMLEditor use GetElementOrParentByTagNameInternal() or GetElementOrParentByTagNameAtSelection() directly. Then, public method, GetElementOrParentByTagName() is called only by outer classes. Note that some callers use both GetElementOrParentByTagNameInternal() and GetElementOrParentByTagNameAtSelection() since they don't check whether setting node is nullptr. They may be bug of them. We should investigate the API callers later.
a28cf4300f126a9ae4960035c991d9280c9830d6: Bug 1483144 - Make HTMLEditor::GetSelectionContainer() protected r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 16 Aug 2018 15:12:51 +0000 - rev 829924
Push 118801 by bmo:rbartlensky@mozilla.com at Fri, 17 Aug 2018 12:50:32 +0000
Bug 1483144 - Make HTMLEditor::GetSelectionContainer() protected r=m_kato HTMLEditor::GetSelectionContainer() is a public method, but it's not used by outer classes. So, we can make it a protected member. Additionally, this patch cleans up the method. - Renames to GetSelectionContainerElement() for making clearer what will be returned. - Makes it const. - Makes it take Selection reference since most callers already have Selection. - Makes it use RangeBoundary to access start point and end point of range since nsRange::StartOffset() and nsRange::EndOffset() may be slow. - Makes it not use GetSelectedElement() since it requires unnecessary additional cost and the condition to call it means it uses only the first path in GetSelectedElement() which just returns start node of the range. - Makes it output warning when it returns nullptr since it reaches nullptr only when illegal cases, e.g., Selection is in orphan node. Differential Revision: https://phabricator.services.mozilla.com/D3461
fd90e385d65bc4eeb960adaae509458504bbead8: Bug 1483132 - Make EditorBase::AreNodesSameType() non-virtual r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 16 Aug 2018 10:29:20 +0000 - rev 829923
Push 118801 by bmo:rbartlensky@mozilla.com at Fri, 17 Aug 2018 12:50:32 +0000
Bug 1483132 - Make EditorBase::AreNodesSameType() non-virtual r=m_kato EditorBase::AreNodesSameType() is overridden only by HTMLEditor and the implementation is enough simple to re-implement in EditorBase. Additionally, this is called from condition of a loop in JoinNodesDeepWithTransaction(). So, the virtual call cost may make damage to the performance. Differential Revision: https://phabricator.services.mozilla.com/D3460
ef882e3e4b6bb67e0bfef77939d1bd1addc68998: Bug 1483127 - Use NS_IMETHODIMP at definition of HTMLEditor::SetIsCSSEnabled() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 16 Aug 2018 10:05:06 +0000 - rev 829922
Push 118801 by bmo:rbartlensky@mozilla.com at Fri, 17 Aug 2018 12:50:32 +0000
Bug 1483127 - Use NS_IMETHODIMP at definition of HTMLEditor::SetIsCSSEnabled() r=m_kato HTMLEditor::SetIsCSSEnabled() is an XPCOM but it's defined with nsresult. Differential Revision: https://phabricator.services.mozilla.com/D3459
c7cba947e5046220f9a3678b465df6db769c58f7: Bug 1483119 - Get rid of HTMLEditor::GetURLForStyleSheet() since unused r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 16 Aug 2018 10:03:49 +0000 - rev 829921
Push 118801 by bmo:rbartlensky@mozilla.com at Fri, 17 Aug 2018 12:50:32 +0000
Bug 1483119 - Get rid of HTMLEditor::GetURLForStyleSheet() since unused r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D3458
f3e7a7dd30a59ebe893fe37caaeb63e63485127d: Bug 1482023 - Create HTMLEditor::EnableStyleSheetInternal() for internal use r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 16 Aug 2018 10:03:46 +0000 - rev 829920
Push 118801 by bmo:rbartlensky@mozilla.com at Fri, 17 Aug 2018 12:50:32 +0000
Bug 1482023 - Create HTMLEditor::EnableStyleSheetInternal() for internal use r=m_kato HTMLEditor::EnableStyleSheet() is an XPCOM method but it's used internally. Therefore, we should create non-virtual method for internal use. Differential Revision: https://phabricator.services.mozilla.com/D3456
4c023e0cd20fde4784d6d974718f659b115ab7de: Bug 1482022 - Create HTMLEditor::RemoveOverrideStyleSheetInternal() for internal use r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 16 Aug 2018 10:01:23 +0000 - rev 829919
Push 118801 by bmo:rbartlensky@mozilla.com at Fri, 17 Aug 2018 12:50:32 +0000
Bug 1482022 - Create HTMLEditor::RemoveOverrideStyleSheetInternal() for internal use r=m_kato HTMLEditor::RemoveOverrideStyleSheet() is an XPCOM method but used internally. So, we should create non-virtual method for this. Additionally, it calls GetStyleSheetForURL() and RemoveStyleSheetFromList(), but they search index of internal override style sheet array redundantly. Moreover, RemoveStyleSheetFromList() returns error only when given URL is not found, but RemoveOverrideStyleSheet() which is the only one caller, ignores the error. Therefore, for saving the redundant cost, this patch makes RemoveStyleSheetFromList() return removing StyleSheet which is retrieved with the call of GetStyleSheetForURL(). So, RemoveOverrideStyleSheetInternal() stops calling GetStyleSheetForURL(). Differential Revision: https://phabricator.services.mozilla.com/D3455
2d5b4c59078e5fb0919ffeabb955b9f78c37174c: Bug 1482021 - Create HTMLEditor::AddOverrideStyleSheetInternal() for internal use r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 16 Aug 2018 09:26:09 +0000 - rev 829915
Push 118801 by bmo:rbartlensky@mozilla.com at Fri, 17 Aug 2018 12:50:32 +0000
Bug 1482021 - Create HTMLEditor::AddOverrideStyleSheetInternal() for internal use r=m_kato HTMLEditor::AddOverrideStyleSheet() is an XPCOM method but it's called internally. So, we should create non-virtual method for it and call it for internal use. Differential Revision: https://phabricator.services.mozilla.com/D3454
fd58d3f130790f6885c474d4f65e743c06d8341e: Bug 1449564 - part 4: Make users can show Gecko specific editing UIs with new prefs r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 16 Aug 2018 13:51:36 +0900 - rev 829867
Push 118799 by masayuki@d-toybox.com at Fri, 17 Aug 2018 05:00:59 +0000
Bug 1449564 - part 4: Make users can show Gecko specific editing UIs with new prefs r?m_kato Even after we disable Gecko specific editing UIs by default, web apps can enable them with execCommand. However, until such web apps change their behavior, users cannot use Gecko specific UIs. At least for now, we should make users can enable them by default. MozReview-Commit-ID: AuAdw4FQ4He
28654839c43dc9ed6754b711e6996a3c05d122f1: Bug 1449564 - part 3: Make absolute position editor listen to mouse events at the system event group r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Thu, 05 Apr 2018 00:32:32 +0900 - rev 829866
Push 118799 by masayuki@d-toybox.com at Fri, 17 Aug 2018 05:00:59 +0000
Bug 1449564 - part 3: Make absolute position editor listen to mouse events at the system event group r?m_kato Currently, absolute position editor listens to mouse events at the default event group to handle dragging of positioner. However, this is blocked by a call of Event.stopPropagation() in web apps unexpectedly. Therefore, we should make it listen to the events at the system event group instead. MozReview-Commit-ID: Hoa8c9QvMuG
6550580c4d4ef1fccb7f0adc34ae7ec2a004a2f0: Bug 1449564 - part 2: Make absolute positioned element editor disabled in default and make it possible to enable it with new command r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 04 Apr 2018 22:27:49 +0900 - rev 829865
Push 118799 by masayuki@d-toybox.com at Fri, 17 Aug 2018 05:00:59 +0000
Bug 1449564 - part 2: Make absolute positioned element editor disabled in default and make it possible to enable it with new command r?m_kato We have another built-in UI of editor which is not implemented by any other browsers. That is a draggable handler to move absolute positioned elements. So, we should disable it in default for compatibility with the other browsers. However, different from resizers and inline table editor, we don't have command to enable/disable this feature but for backward compatibility, we should have it. Therefore, this patch adds new command "enableAbsolutePositionEditor". Note that whether resizing UI is available only with enableObjectResizing state is different from enableInlineTableEditing command. Resizers for absolute positioned elements are NOT available both enableObjectResizing and enableAbsolutePositionEditor are enabled. Additionally, this adds automated tests to check basic functions of absolute positioned editor. MozReview-Commit-ID: 9ZSGB8tLpFw
6fd49c76a5ec643a7d8346971a918904d3d9a266: Bug 1449564 - part 1: Disable object resizer and inline table editor in default r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 02 Apr 2018 17:26:46 +0900 - rev 829864
Push 118799 by masayuki@d-toybox.com at Fri, 17 Aug 2018 05:00:59 +0000
Bug 1449564 - part 1: Disable object resizer and inline table editor in default r?m_kato Gecko supports resizers of <img> elements and <table>, <td>, <th> elements and has UI to remove existing table row or column in default. However, the other browsers don't have such UI and web apps need to disable this feature with calling both: document.execCommand("enableObjectResizing", false, false); document.execCommand("enableInlineTableEditing", false, false); for avoiding conflicting with their own features to edit such elements. Therefore, it doesn't make sense to keep enabling them in default only on Gecko. If web apps want to keep using these features, they should call: document.execCommand("enableObjectResizing", false, true); document.execCommand("enableInlineTableEditing", false, true); at initializing the editor. And also this patch fixes bugs of document.queryCommandState("enableObjectResizing") and document.queryCommandState("enableInlineTableEditing"). They always return false even after calling document.execCommand(..., false, true) since nsSetDocumentStateCommand::GetCommandStateParams() sets bool value as STATE_ATTRIBUTE. However, nsHTMLDocument::QueryCommandValue() which is the caller referring STATE_ATTRIBUTE doesn't treat it as bool value. And also those commands are related to state of document. Therefore, they should be return as bool value of STATE_ALL instead. Then, nsHTMLDocument::QueryCommandState() returns the state as expected. Note that those commands are supported only by Gecko. So, we don't need to worry about the compatibility. Finally, this patch rewrites 2 existing tests to check basic behavior of resizers and appearance of resizers. Note that this patch does not add new tests to test inline table editor since it's difficult to test the behavior with current API. Perhaps, we should add an API to nsIHTMLEditor to retrieve each anonymous elements in another bug since it requires to add wrapping API of SpecialPowers. MozReview-Commit-ID: 1FhYo5vcV60
f8316ce96f9b3ff1993e8ff4a3389e8ae07f4db9: Bug 1483144 - Make HTMLEditor::GetSelectionContainer() protected r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 14 Aug 2018 17:20:04 +0900 - rev 829863
Push 118799 by masayuki@d-toybox.com at Fri, 17 Aug 2018 05:00:59 +0000
Bug 1483144 - Make HTMLEditor::GetSelectionContainer() protected r?m_kato HTMLEditor::GetSelectionContainer() is a public method, but it's not used by outer classes. So, we can make it a protected member. Additionally, this patch cleans up the method. - Renames to GetSelectionContainerElement() for making clearer what will be returned. - Makes it const. - Makes it take Selection reference since most callers already have Selection. - Makes it use RangeBoundary to access start point and end point of range since nsRange::StartOffset() and nsRange::EndOffset() may be slow. - Makes it not use GetSelectedElement() since it requires unnecessary additional cost and the condition to call it means it uses only the first path in GetSelectedElement() which just returns start node of the range. - Makes it output warning when it returns nullptr since it reaches nullptr only when illegal cases, e.g., Selection is in orphan node.
50746ac794b92a18d5c84959652a29b9c9d16ace: Bug 1483132 - Make EditorBase::AreNodesSameType() non-virtual r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 14 Aug 2018 16:25:11 +0900 - rev 829862
Push 118799 by masayuki@d-toybox.com at Fri, 17 Aug 2018 05:00:59 +0000
Bug 1483132 - Make EditorBase::AreNodesSameType() non-virtual r?m_kato EditorBase::AreNodesSameType() is overridden only by HTMLEditor and the implementation is enough simple to re-implement in EditorBase. Additionally, this is called from condition of a loop in JoinNodesDeepWithTransaction(). So, the virtual call cost may make damage to the performance.
be8ca05df3e286b6eade8049345d46c3685bcba8: Bug 1483127 - Use NS_IMETHODIMP at definition of HTMLEditor::SetIsCSSEnabled() r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 14 Aug 2018 16:03:05 +0900 - rev 829861
Push 118799 by masayuki@d-toybox.com at Fri, 17 Aug 2018 05:00:59 +0000
Bug 1483127 - Use NS_IMETHODIMP at definition of HTMLEditor::SetIsCSSEnabled() r?m_kato HTMLEditor::SetIsCSSEnabled() is an XPCOM but it's defined with nsresult.
e474531c276e112f51f29cff5b7d3de88931edca: Bug 1483119 - Get rid of HTMLEditor::GetURLForStyleSheet() since unused r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 14 Aug 2018 15:16:32 +0900 - rev 829860
Push 118799 by masayuki@d-toybox.com at Fri, 17 Aug 2018 05:00:59 +0000
Bug 1483119 - Get rid of HTMLEditor::GetURLForStyleSheet() since unused r?m_kato
0fa34abd263543b35abf7b20ba5cd8aaaafd76be: Bug 1482023 - Create HTMLEditor::EnableStyleSheetInternal() for internal use r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 14 Aug 2018 15:05:24 +0900 - rev 829859
Push 118799 by masayuki@d-toybox.com at Fri, 17 Aug 2018 05:00:59 +0000
Bug 1482023 - Create HTMLEditor::EnableStyleSheetInternal() for internal use r?m_kato HTMLEditor::EnableStyleSheet() is an XPCOM method but it's used internally. Therefore, we should create non-virtual method for internal use.
31e3861d2a5995668734c77da97a5a7f7a86142a: Bug 1482022 - Create HTMLEditor::RemoveOverrideStyleSheetInternal() for internal use r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 14 Aug 2018 14:52:26 +0900 - rev 829858
Push 118799 by masayuki@d-toybox.com at Fri, 17 Aug 2018 05:00:59 +0000
Bug 1482022 - Create HTMLEditor::RemoveOverrideStyleSheetInternal() for internal use r?m_kato HTMLEditor::RemoveOverrideStyleSheet() is an XPCOM method but used internally. So, we should create non-virtual method for this. Additionally, it calls GetStyleSheetForURL() and RemoveStyleSheetFromList(), but they search index of internal override style sheet array redundantly. Moreover, RemoveStyleSheetFromList() returns error only when given URL is not found, but RemoveOverrideStyleSheet() which is the only one caller, ignores the error. Therefore, for saving the redundant cost, this patch makes RemoveStyleSheetFromList() return removing StyleSheet which is retrieved with the call of GetStyleSheetForURL(). So, RemoveOverrideStyleSheetInternal() stops calling GetStyleSheetForURL().
97256d2103b7143292b3d8fdc19016de42d947fa: Bug 1482021 - Create HTMLEditor::AddOverrideStyleSheetInternal() for internal use r?m_kato draft
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 14 Aug 2018 14:21:53 +0900 - rev 829857
Push 118799 by masayuki@d-toybox.com at Fri, 17 Aug 2018 05:00:59 +0000
Bug 1482021 - Create HTMLEditor::AddOverrideStyleSheetInternal() for internal use r?m_kato HTMLEditor::AddOverrideStyleSheet() is an XPCOM method but it's called internally. So, we should create non-virtual method for it and call it for internal use.
417967cf69de08d624ccbc85886ce8f0e224fdd4: Bug 1482020 - Make all callers of CreateElementWithDefaults() use non-virtual method r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 10 Aug 2018 19:36:24 +0900 - rev 829652
Push 118788 by bmo:dharvey@mozilla.com at Thu, 16 Aug 2018 11:52:55 +0000
Bug 1482020 - Make all callers of CreateElementWithDefaults() use non-virtual method r=m_kato Fortunately, despite of becoming public method, HTMLEditor::CreateElementWithDefaults() can be used by internal methods too since it does not touch undo transactions nor the DOM tree, and does not refer mRules nor GetSelection(). So, we can make it public and make any C++ callers use it.