searching for reviewer(m_kato)
aff13d214a35: Bug 1528943 - Stop trying to retrieve raw keycode value when native virtual keycode is VK_PROCESS r=m_kato a=lizzard
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 22 Feb 2019 03:32:48 +0000 - rev 516303
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1528943 - Stop trying to retrieve raw keycode value when native virtual keycode is VK_PROCESS r=m_kato a=lizzard We didn't dispatch keyboard events during composition. Therefore, we tried to retrieve raw virtual keycode value when key messages come with `VK_PROCESS` unexpectedly. However, the API, `ImmGetVirtualKey()`, is not usable for this purpose because it always returns `VK_PROCESS` if the key messages have already been handled by IME. So, we can just stop using it since we need to expose such key messages as KeyboardEvent whose `key` value is `Process` and `keyCode` value is `DOM_VK_PROCESS`. Differential Revision: https://phabricator.services.mozilla.com/D20623
ab901cb2baa5: Bug 1528715 - Work around problems with forcing the autocomplete result's capitalisation. r=geckoview-reviewers,m_kato a=lizzard
Jan Henning <jh+bugzilla@buttercookie.de> - Fri, 22 Feb 2019 13:11:32 +0000 - rev 516179
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1528715 - Work around problems with forcing the autocomplete result's capitalisation. r=geckoview-reviewers,m_kato a=lizzard Even though using text.replace() is supposed to keep the existing spans intact, in practice the composition spans still get lost, even when the replacement text is identical (because there was in fact no capitalisation difference between user-entered text and autocomplete result). One approach to fix this would be to manually save the composition spans and subsequently restore them afterwards, like we already do this a few lines further down below, in the other major code path through onAutocomplete(). However while trying to generalise that approach, the most natural approach for the caller to specify which spans it wanted to save was to pass a Predicate lambda to the state saving function, which for some reason led to strange "Didn't find class" errors. So instead, we just restart input for affected IMEs (i.e. Sony's keyboard) to get them back into sync with the new contents of the EditText. To avoid unnecessarily calling restartInput(), though, we only do this if the user-entered text (which at this stage is known to correspond to the auto- complete result when compared case-*insensitively*) actually differs from the autocomplete result. Differential Revision: https://phabricator.services.mozilla.com/D20487
af029ffaf0ff: Bug 1529467 - Make TextInputHandler::HandleCommand() dispatch eKeyDown event r=m_kato a=lizzard
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 25 Feb 2019 04:13:20 +0000 - rev 516163
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1529467 - Make TextInputHandler::HandleCommand() dispatch eKeyDown event r=m_kato a=lizzard Currently, when IME sends a command with committing composition, TextInputHandler::HandleCommand() dispatches a *fake* `keypress` event for making default event handler (e.g., focused editor) handle the event. However, we stopped dispatching `keypress` events for non-printable keys. Therefore, web apps cannot do that like us. On macOS, simple conversion IMEs like Korean 2-set IME, behave differently if active application is changed. E.g., on Safari, some of them may never use composition string, but not so on Chrome and Firefox. So, this is what the case we need to emulate Safari's behavior. Dispatching a fake `keydown` event for this purpose does **not** conform to UI Events because `keydown` events should notify web apps of **physical** key state changes. However, Chrome dispatches fake `keydown` events intentionally. Therefore, we should follow this hacky behavior for user experience. Differential Revision: https://phabricator.services.mozilla.com/D20644
380761f22d52: Bug 1523635 - part 2: Make IMContextWrapper::OnKeyEvent() trust the result of gtk_im_context_filter_keypress() to decide whether handling event causes async event later r=m_kato a=RyanVM
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 05 Feb 2019 11:59:38 +0000 - rev 515783
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1523635 - part 2: Make IMContextWrapper::OnKeyEvent() trust the result of gtk_im_context_filter_keypress() to decide whether handling event causes async event later r=m_kato a=RyanVM Unfortunately, we're not sure whether ibus always handles non-dead key events asynchronously or synchoronously. Therefore, for safer fix, this patch makes IMContextWrapper::OnKeyEvent() decide that with the result of gtk_im_context_filter_keypress(). If active IME is ibus and it consumes non- synthesized events on password fields, it adjusts probablyHandledAsynchronously value. So, this patch changes the behavior of only when: - active IME is ibus. - only when a password field or ime-mode:disabled field has focus. - not in dead key sequence. - and the key event is consumed by ibus but nothing happend. This must be enough safe to uplift. Differential Revision: https://phabricator.services.mozilla.com/D18635
f950041314ee: Bug 1523635 - part 1: Rename `maybeHandledAsynchronously` to `probablyHandledAsynchronously` r=m_kato a=RyanVM
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 05 Feb 2019 06:48:08 +0000 - rev 515782
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1523635 - part 1: Rename `maybeHandledAsynchronously` to `probablyHandledAsynchronously` r=m_kato a=RyanVM Now, we believe that when `maybeHandledAsynchronously` is set to true, ibus handles the event asynchronously in usual cases. However, the behavior of ibus on password field is unclear. Currently, on Ubuntu 18.04, Ubuntu 18.10 and Debian Cinnamon (9.6 / 3.2.7), ibus handles key events asynchronously even in password fields even though I confirmed it was not so at initial fix. So, it could be just my mistake, but we need to prepare for both cases here for safer fix. So, in the following patch, I need to add another variable for weaker decision, and we treat `maybeHandledAsynchronously` stronger than its nuance. Therefore, this patch renames it to `probablyHandledAsynchronously`. Differential Revision: https://phabricator.services.mozilla.com/D18634
39296d5b2c92: Bug 1517937 - Move JhengHei to the end of zh-TW sans-serif font prefs on late-beta/release so that previous default of PMingLiu will take precedence for now. r=m_kato
Jonathan Kew <jkew@mozilla.com> - Fri, 18 Jan 2019 18:16:42 +0000 - rev 514596
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1517937 - Move JhengHei to the end of zh-TW sans-serif font prefs on late-beta/release so that previous default of PMingLiu will take precedence for now. r=m_kato
876faba9c48f: Bug 1515351 - Update encoding_rs to 0.8.14. r=m_kato
Henri Sivonen <hsivonen@hsivonen.fi> - Thu, 10 Jan 2019 09:42:39 +0000 - rev 513218
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1515351 - Update encoding_rs to 0.8.14. r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D15934
5665c5f25198: Bug 1482659 - Purge the last sound, free then start to play a sound r=m_kato,masayuki
Thomas Nguyen <tnguyen@mozilla.com> - Wed, 09 Jan 2019 12:26:46 +0000 - rev 513179
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1482659 - Purge the last sound, free then start to play a sound r=m_kato,masayuki Because we don't know when the PlaySound API stop/finish playing. So when starting to play a sound, we should call PlaySound(0,0,PURGE) to stop playing then free the last data sound. Differential Revision: https://phabricator.services.mozilla.com/D14554
a43422e9e4da: Bug 1518401 - Exclude caret position within ligated emoji sequence where the second component was a BMP symbol + VS16. r=m_kato
Jonathan Kew <jkew@mozilla.com> - Wed, 09 Jan 2019 10:13:40 +0000 - rev 513022
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1518401 - Exclude caret position within ligated emoji sequence where the second component was a BMP symbol + VS16. r=m_kato
e80f4801231c: Bug 1516326 - part 2: Log more in KeymapWrapper r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 07 Jan 2019 10:05:56 +0000 - rev 512779
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1516326 - part 2: Log more in KeymapWrapper r=m_kato We haven't logged KeymapWrapper::HandleKeyPressEvent() nor KeymapWrapper::HandleKeyReleaseEvent(). Therefore, this patch makes them put their behavior into the log. Additionally, KeymapWrapper::InitKeyEvent() does not log enough the result of initialized WidgetkeyboardEvent. This patch makes it log more. With those changes, we can get the log of: - detail of dispatched keyboard events - which key event didn't cause keyboard events - which keyboard event was consumed Note that the utility methods are copied from widget/windows. Perhaps, we should create XP logging helper for keyboard/IME later. Differential Revision: https://phabricator.services.mozilla.com/D15324
11963e8ad79f: Bug 1498823 - Make KeymapWrapper::FilterEvents() ignore synthesized KeyPress events r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 28 Dec 2018 07:02:05 +0000 - rev 512017
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1498823 - Make KeymapWrapper::FilterEvents() ignore synthesized KeyPress events r=m_kato With an event filter method, we're logging all key events on any widgets to check whether a key is pressed or not, to set WidgetKeyboardEvent::mIsRepeat properly. If iBus and Fcitx work as expected, they synthesize key events with setting their own modifier state which indicate the events are synthesized by IME. In this expected case, synthesized key events are not caught by the filter. On the other hand, in some environment, they keep handling key events asynchronously but they or something another module synthesizes key events without the flag and such events are caught by the filter because the events are posted into the event queue. Therefore, we decide that such synthesized events are always generated by auto-repeat (first events which are always filtered by IME are treated as first press, and then, synthesized events are treated as repeated events because of no key release events). This patch makes KeymapWrapper::FilterEvents() ignore coming KeyPress events if: - the time is exactly same as previous KeyPress event - and IMContextWrapper instance is now waiting a GDK_KEY_PRESS event - and hardware_keycode of waiting GDK_KEY_PRESS event is same as keyCode of the KeyPress event Differential Revision: https://phabricator.services.mozilla.com/D15380
90370fd4bc2a: Bug 1516323 - Make IMContextWrapper::OnKeyEvent() check whether coming key event is already in the posting event queue r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 26 Dec 2018 08:17:32 +0000 - rev 511909
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1516323 - Make IMContextWrapper::OnKeyEvent() check whether coming key event is already in the posting event queue r=m_kato According to the log of bug 1498823, ibus won't set IBUS_IGNORED_MASK to modifier flags when it synthesizes the event for asynchronous handling in some environments. Currently, we assume that iBus and Fcitx set IBUS_IGNORED_MASK or FcitxKeyState_IgnoredMask. So, we put both real key events and synthesized key events into the posting event queue and that causes using a lot of memory until the editor is blurred. Fortunately, timestamp of synthesized key events are always same as their original events. Therefore, we can look for original event from the positing event queue. Although we have gotten no bug reports about this issue of Fcitx, but this patch adds same hack for Fcitx too because the runtime cost is not expensive but the symptom is really serious. Differential Revision: https://phabricator.services.mozilla.com/D15321
562edbba63e6: Bug 1514726 - Avoid an allocation and copy in TextEncoder.encode() in the non-ASCII case. r=m_kato
Henri Sivonen <hsivonen@hsivonen.fi> - Tue, 18 Dec 2018 11:40:31 +0000 - rev 511132
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1514726 - Avoid an allocation and copy in TextEncoder.encode() in the non-ASCII case. r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D14728
e3deacdccfe5: Bug 1504963 - part 4: Make IMEHandler create native caret over our caret if it's necessary r=Jamie,m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 18 Dec 2018 08:38:23 +0000 - rev 511095
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1504963 - part 4: Make IMEHandler create native caret over our caret if it's necessary r=Jamie,m_kato IMMHandler and TSFTextStore are good class to put native caret when they have enough information. However, for example, IMMHandler may not have its singleton instance until first composition of IMM-IME starts. Therefore, typically, IMEHandler is a good class to put native caret without composition. This patch adds IMEHandler::MaybeCreateNativeCaret(), and if it won't create native caret because not yet received WM_GETOBJCT for OBJID_CARET, we should fire window event for MSAA applications. If there is new MSAA application retrieves OBJID_CARET, we'll receive WM_GETOBJECT for OBJID_CARET asynchronously. Then, we should start to put native caret for such applications. Note that if we create native caret, some versions of ATOK refers the native caret and the behavior becomes worse than usual. Therefore, we need to keep not using native caret as far as possible. Differential Revision: https://phabricator.services.mozilla.com/D13962
0c8ede3250a6: Bug 1504963 - part 3: Make TSFTextStore create native caret when it gets notified of content change r=Jamie,m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 18 Dec 2018 08:38:23 +0000 - rev 511094
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1504963 - part 3: Make TSFTextStore create native caret when it gets notified of content change r=Jamie,m_kato If WM_GETOBJECT for OBJID_CARET is received but a11y module is not active, IME module should create native caret over our caret because Windows will handle the request with native caret automatically and we don't need to enable a11y module only for it. This patch makes IMEHandler store whether such message has been received and makes TSFTextStore create native caret when composition, selection or layout is changed because especially when there is composition, only TSFTextStore knows correct position to put caret if there is composition or some dispatched events have not been handled by content process yet. Note that IMMHandler already does that since some legacy IMEs require native caret to show its UI and we cannot check active IME strictly. Therefore, this patch does not touch IMMHandler. Differential Revision: https://phabricator.services.mozilla.com/D13961
67359f421e75: Bug 1504963 - part 2: Make IMEHandler manage whether native caret is created by it r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 18 Dec 2018 08:38:22 +0000 - rev 511093
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1504963 - part 2: Make IMEHandler manage whether native caret is created by it r=m_kato IMEHandler needs to create native caret later (when there is no composition). Therefore, IMEHandler should manage whether it creates native caret or not and IMMHandler and TSFTextStore should create/destroy native caret via IMEHandler. Note that this patch makes IMMHandler stops managing whether native caret is created for plugin or not because native caret is created only one instance and anyway IME handlers should stop managing native caret when they loses focus. Differential Revision: https://phabricator.services.mozilla.com/D13960
beecf3381657: Bug 1504963 - part 1: Make IME modules not touch native caret if a11y module handles native caret r=Jamie,m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 18 Dec 2018 10:40:50 +0000 - rev 511092
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1504963 - part 1: Make IME modules not touch native caret if a11y module handles native caret r=Jamie,m_kato If a11y module is active, it observers caret position and size, and when caret position or size is changed, it creates/moves native caret to overlap with our caret. On the other hand, IME module also creates native caret if active IME requires it. Therefore, both of them conflicts each other. This patch makes IME module stop touching native caret if a11y module is active. Although, a11y module with Flush Player does not work well for IME. Therefore, this patch keeps the conflict between them as-is for now. Differential Revision: https://phabricator.services.mozilla.com/D13959
62a30f449215: Bug 1513145 - Make some callers of TSFTextStore::Selection::GetWritingMode() check whether the selection has already been initialized r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Sat, 15 Dec 2018 03:23:38 +0000 - rev 510876
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1513145 - Make some callers of TSFTextStore::Selection::GetWritingMode() check whether the selection has already been initialized r=m_kato If TSFTextStore fails to get selection, e.g., the content is really odd like fuzzing tests, its mSelectionForTSF is marked as dirty. However, Windows may try to retrieve writing mode while we're creating new TSFTextStore. Then, we may hit MOZ_ASSERT(!mDirty) in TSFTextStore::Selection::GetWritingMode() in debug build. So, we need to make some callers of GetWritingMode() check whether selection is marked as dirty. Differential Revision: https://phabricator.services.mozilla.com/D14137
a126bd4f1ae9: Bug 1512214 - Use promise chaining. r=m_kato
Jean-Yves Avenard <jyavenard@mozilla.com> - Fri, 07 Dec 2018 17:47:19 +0000 - rev 510094
Push 1953 by ffxbld-merge at Mon, 11 Mar 2019 12:10:20 +0000
Bug 1512214 - Use promise chaining. r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D13952
55b9be64fd88: Bug 1513145 - Make some callers of TSFTextStore::Selection::GetWritingMode() check whether the selection has already been initialized. r=m_kato, a=RyanVM
Masayuki Nakano <masayuki@d-toybox.com> - Sat, 15 Dec 2018 03:23:38 +0000 - rev 509634
Push 1934 by ryanvm@gmail.com at Mon, 11 Feb 2019 15:05:00 +0000
Bug 1513145 - Make some callers of TSFTextStore::Selection::GetWritingMode() check whether the selection has already been initialized. r=m_kato, a=RyanVM If TSFTextStore fails to get selection, e.g., the content is really odd like fuzzing tests, its mSelectionForTSF is marked as dirty. However, Windows may try to retrieve writing mode while we're creating new TSFTextStore. Then, we may hit MOZ_ASSERT(!mDirty) in TSFTextStore::Selection::GetWritingMode() in debug build. So, we need to make some callers of GetWritingMode() check whether selection is marked as dirty. Differential Revision: https://phabricator.services.mozilla.com/D14137
c48e0ec8db4f: Bug 1523635 - part 2: Make IMContextWrapper::OnKeyEvent() trust the result of gtk_im_context_filter_keypress() to decide whether handling event causes async event later r=m_kato a=RyanVM
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 05 Feb 2019 11:59:38 +0000 - rev 509626
Push 1932 by ryanvm@gmail.com at Fri, 08 Feb 2019 22:53:06 +0000
Bug 1523635 - part 2: Make IMContextWrapper::OnKeyEvent() trust the result of gtk_im_context_filter_keypress() to decide whether handling event causes async event later r=m_kato a=RyanVM Unfortunately, we're not sure whether ibus always handles non-dead key events asynchronously or synchoronously. Therefore, for safer fix, this patch makes IMContextWrapper::OnKeyEvent() decide that with the result of gtk_im_context_filter_keypress(). If active IME is ibus and it consumes non- synthesized events on password fields, it adjusts probablyHandledAsynchronously value. So, this patch changes the behavior of only when: - active IME is ibus. - only when a password field or ime-mode:disabled field has focus. - not in dead key sequence. - and the key event is consumed by ibus but nothing happend. This must be enough safe to uplift. Differential Revision: https://phabricator.services.mozilla.com/D18635
e58952a2b64a: Bug 1523635 - part 1: Rename `maybeHandledAsynchronously` to `probablyHandledAsynchronously` r=m_kato a=RyanVM
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 05 Feb 2019 06:48:08 +0000 - rev 509625
Push 1932 by ryanvm@gmail.com at Fri, 08 Feb 2019 22:53:06 +0000
Bug 1523635 - part 1: Rename `maybeHandledAsynchronously` to `probablyHandledAsynchronously` r=m_kato a=RyanVM Now, we believe that when `maybeHandledAsynchronously` is set to true, ibus handles the event asynchronously in usual cases. However, the behavior of ibus on password field is unclear. Currently, on Ubuntu 18.04, Ubuntu 18.10 and Debian Cinnamon (9.6 / 3.2.7), ibus handles key events asynchronously even in password fields even though I confirmed it was not so at initial fix. So, it could be just my mistake, but we need to prepare for both cases here for safer fix. So, in the following patch, I need to add another variable for weaker decision, and we treat `maybeHandledAsynchronously` stronger than its nuance. Therefore, this patch renames it to `probablyHandledAsynchronously`. Differential Revision: https://phabricator.services.mozilla.com/D18634
9876a3bd6201: Bug 1517937 - Move JhengHei to the end of zh-TW sans-serif font prefs on late-beta/release so that previous default of PMingLiu will take precedence for now. r=m_kato, a=RyanVM FENNEC_65_0b13_BUILD1 FENNEC_65_0b13_RELEASE FIREFOX_RELEASE_65_BASE
Jonathan Kew <jkew@mozilla.com> - Fri, 18 Jan 2019 18:16:42 +0000 - rev 509556
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1517937 - Move JhengHei to the end of zh-TW sans-serif font prefs on late-beta/release so that previous default of PMingLiu will take precedence for now. r=m_kato, a=RyanVM
c4bda79261ba: Bug 1482659 - Purge the last sound, free then start to play a sound. r=m_kato,masayuki, a=RyanVM
Thomas Nguyen <tnguyen@mozilla.com> - Wed, 09 Jan 2019 12:26:46 +0000 - rev 509390
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1482659 - Purge the last sound, free then start to play a sound. r=m_kato,masayuki, a=RyanVM Because we don't know when the PlaySound API stop/finish playing. So when starting to play a sound, we should call PlaySound(0,0,PURGE) to stop playing then free the last data sound. Differential Revision: https://phabricator.services.mozilla.com/D14554
d3c013ec52f1: Bug 1498823 - Make KeymapWrapper::FilterEvents() ignore synthesized KeyPress events. r=m_kato, a=RyanVM
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 28 Dec 2018 07:02:05 +0000 - rev 509208
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1498823 - Make KeymapWrapper::FilterEvents() ignore synthesized KeyPress events. r=m_kato, a=RyanVM With an event filter method, we're logging all key events on any widgets to check whether a key is pressed or not, to set WidgetKeyboardEvent::mIsRepeat properly. If iBus and Fcitx work as expected, they synthesize key events with setting their own modifier state which indicate the events are synthesized by IME. In this expected case, synthesized key events are not caught by the filter. On the other hand, in some environment, they keep handling key events asynchronously but they or something another module synthesizes key events without the flag and such events are caught by the filter because the events are posted into the event queue. Therefore, we decide that such synthesized events are always generated by auto-repeat (first events which are always filtered by IME are treated as first press, and then, synthesized events are treated as repeated events because of no key release events). This patch makes KeymapWrapper::FilterEvents() ignore coming KeyPress events if: - the time is exactly same as previous KeyPress event - and IMContextWrapper instance is now waiting a GDK_KEY_PRESS event - and hardware_keycode of waiting GDK_KEY_PRESS event is same as keyCode of the KeyPress event Differential Revision: https://phabricator.services.mozilla.com/D15380
4cb9d2d57e7d: Bug 1516323 - Make IMContextWrapper::OnKeyEvent() check whether coming key event is already in the posting event queue. r=m_kato, a=RyanVM
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 26 Dec 2018 08:17:32 +0000 - rev 509206
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1516323 - Make IMContextWrapper::OnKeyEvent() check whether coming key event is already in the posting event queue. r=m_kato, a=RyanVM According to the log of bug 1498823, ibus won't set IBUS_IGNORED_MASK to modifier flags when it synthesizes the event for asynchronous handling in some environments. Currently, we assume that iBus and Fcitx set IBUS_IGNORED_MASK or FcitxKeyState_IgnoredMask. So, we put both real key events and synthesized key events into the posting event queue and that causes using a lot of memory until the editor is blurred. Fortunately, timestamp of synthesized key events are always same as their original events. Therefore, we can look for original event from the positing event queue. Although we have gotten no bug reports about this issue of Fcitx, but this patch adds same hack for Fcitx too because the runtime cost is not expensive but the symptom is really serious. Differential Revision: https://phabricator.services.mozilla.com/D15321
f7fafed0e4a9: Bug 1511752 - Make IMContextWrapper::OnKeyEvent() treat GDK_KEY_PRESS event without any key information in a dead key sequence as synthesized by current keyboard layout for asynchronous handling r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 03 Dec 2018 15:15:13 +0000 - rev 508494
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1511752 - Make IMContextWrapper::OnKeyEvent() treat GDK_KEY_PRESS event without any key information in a dead key sequence as synthesized by current keyboard layout for asynchronous handling r=m_kato Some keyboard layouts which have dead keys may handle dead key composition asynchronously. In this case, they don't rely on IME like iBus/Fcitx. As far as I've tested, German (QWERTY) keyboard layout is one of such keyboard layout. This returns true when gtk_im_context_filter_keypress() is called for a base character input (like "a"). Then, it synthesizes GDK_KEY_PRESS event without any key information such as: > { type=GDK_KEY_PRESS, keyval=(null), unicode=0x0, state=, time=0, > hardware_keycode=0, group=0 } So, this is not marked as IBUS_IGNORED_MASK nor FcitxKeyState_IgnoredMask by IME, but we should ignore this event since we should've already dispatched "keydown" event for the preceding "a" key event, and anyway "keydown" event for the synthesized event does not make sense for any web apps. This patch makes IMContextWrapper::OnKeyEvent() ignore such key event, i.e., when it's in a dead key sequence, and GDK_KEY_PRESS does not have enough information, e.g., hardware_keycode shouldn't be 0 especially for printable keys. Therefore, this patch make it check only hardware_keycode value and gdk_keyval_to_unicode(aEvent->keyval) value. If some keyboard layouts would send the original key event as is, we would need to do more, but currently, this is enough and safe to land this timing. Differential Revision: https://phabricator.services.mozilla.com/D13656
5e2fad03c885: Bug 1507543 - Spellchecker for contenteditable/design-mode should not run without focus; r=m_kato
Edgar Chen <echen@mozilla.com> - Mon, 03 Dec 2018 11:20:09 +0000 - rev 508446
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1507543 - Spellchecker for contenteditable/design-mode should not run without focus; r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D12875
e9eedbf1bd4b: Bug 1509507 - Update to encoding_rs 0.8.13 to fix a panic in UTF-8 to UTF-16 decode. r=m_kato
Henri Sivonen <hsivonen@hsivonen.fi> - Fri, 30 Nov 2018 07:42:26 +0000 - rev 508182
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1509507 - Update to encoding_rs 0.8.13 to fix a panic in UTF-8 to UTF-16 decode. r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D13253
b096dc953832: Bug 1510183 - Make HTMLEditor treat empty string attribute of style as nullptr of nsAtom rather than nsGkAtoms::_empty r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 30 Nov 2018 01:21:59 +0000 - rev 508107
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1510183 - Make HTMLEditor treat empty string attribute of style as nullptr of nsAtom rather than nsGkAtoms::_empty r=m_kato After fixing bug 1427060, HTMLEditor treats attribute of style as nullptr. However, if empty string is used to call NS_Atomize(), it returns nsGkAtoms::_empty. Therefore, HTMLEditor fails to check whether attribute is specified or not with nullptr check since some root callers sets nsGkAtoms::_empty instead of nullptr. This patch makes HTMLEditor always use nullptr for empty string of attribute of style with wrapping NS_Atomize() with AtomzieAttribute(). Additionally, for safer change, this patch makes PropItem and TypeInState treat nsGkAtom::_empty as nullptr. Differential Revision: https://phabricator.services.mozilla.com/D13203
6a0012913373: Bug 1503157 - Make selection of font preferences respect a script subtag in the lang attribute, if present. r=m_kato
Jonathan Kew <jkew@mozilla.com> - Mon, 26 Nov 2018 11:31:37 +0000 - rev 507192
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1503157 - Make selection of font preferences respect a script subtag in the lang attribute, if present. r=m_kato
f17b7ba6d0aa: Bug 1505147 - nsWindow::OnKeyPressEvent() shouldn't dispatch eKeyDown event when IMContextWrapper::OnKeyEvent() has already dispatched it for the event r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 26 Nov 2018 03:26:39 +0000 - rev 507172
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1505147 - nsWindow::OnKeyPressEvent() shouldn't dispatch eKeyDown event when IMContextWrapper::OnKeyEvent() has already dispatched it for the event r=m_kato Currently, IMContextWrapper::OnKeyEvent() assumes that IME won't synthesize keyboard event asynchronously again in some cases. For example, one of the cases is that user inputs text with a dead key sequence. However, IME may synthesize key event asynchronously only in a few cases even in a dead key sequence. Unfortunately, for not losing a chance to dispatch eKeyDown/eKeyUp event, we need to keep dispatching eKeyDown or eKeyUp event when we receive original event in dead key sequence. However, according to this bug, we need to stop dispatching eKeyDown and eKeyUp events when we receive unexpected async synthesized key event. If IMContextWrapper::OnKeyEvent() needs to return whether it (has already) dispatched an eKeyDown or eKeyUp and whether it was consumed, then, nsWindow can stop dispatching redundant eKeyDown and eKeyUp events. So, this patch makes IMContextWrapper::OnKeyEvent() return KeyHandlingState enum class instead of just a bool value to notify the caller of detail of the event status. And also makes each caller of nsWindow not dispatch eKeyDown nor eKeyUp event when it returns KeyHandlingState::eNotHandledButDispatched or KeyHandlingState::eNotHandledButConsumed. Differential Revision: https://phabricator.services.mozilla.com/D12517
fb45231e386d: Bug 1497746 - part 6: Add static_assert for preventing TextEditor to grow up again r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 26 Nov 2018 06:33:28 +0000 - rev 507169
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1497746 - part 6: Add static_assert for preventing TextEditor to grow up again r=m_kato TextEditor instance is created per <input> element which has text editor and <textarea> element. Therefore, we should keep TextEditor slim as far as possible. Currently, TextEditor class size is 400 bytes on Win64. So, we should keep 512 bytes border. Differential Revision: https://phabricator.services.mozilla.com/D12404
324232a1330c: Bug 1497746 - part 5: Make EditorBase not reserve array for its listeners unless listeners are important r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 26 Nov 2018 06:32:34 +0000 - rev 507168
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1497746 - part 5: Make EditorBase not reserve array for its listeners unless listeners are important r=m_kato A lot of listeners are now notified with RefPtr for concrete classes. Therefore, we can reduce size of arrays to listeners without damage for the performance. Differential Revision: https://phabricator.services.mozilla.com/D12403
ceb75e2e0421: Bug 1497746 - part 4: Move EditorBase::mRangeUpdater to AutoEditActionDataSetter r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 26 Nov 2018 06:31:56 +0000 - rev 507167
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1497746 - part 4: Move EditorBase::mRangeUpdater to AutoEditActionDataSetter r=m_kato Similar to EditorBase::mSavedSel, we can move EditorBase::mRangeUpdater too because of it's referred only when there is AutoEditActionDataSetter instance so that it also does not need to be in the cycle collection. And now, it can be marked as MOZ_STACK_CLASS and remove cycle collection support. Differential Revision: https://phabricator.services.mozilla.com/D12402
465ebb044ee1: Bug 1497746 - part 3: Move EditorBase::mSavedSel into AutoEditActionDataSetter r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 26 Nov 2018 06:31:13 +0000 - rev 507166
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1497746 - part 3: Move EditorBase::mSavedSel into AutoEditActionDataSetter r=m_kato EditorBase::mSavedSel is used only by EditorBase methods which are called only by AutoSelectionRestorer. Additionally, AutoSelectionRestorer requires AutoEditActionDataSetter instance. So, we don't need to keep create for editor instance anymore. And also we don't need to keep it in the cycle collection. Note that SelectionState class is also used by PlaceholderTransaction. Therefore, we cannot make it MOZ_STACK_CLASS. Differential Revision: https://phabricator.services.mozilla.com/D12401
097a6db49477: Bug 1497746 - part 2: Move EditorBase::mDirection to EditorBase::AutoEditActionDataSetter r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 26 Nov 2018 06:30:29 +0000 - rev 507165
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1497746 - part 2: Move EditorBase::mDirection to EditorBase::AutoEditActionDataSetter r=m_kato EditorBase::mDirection is set and clear only when EditorBase::AutoEditActionDataSetter::SetTopLevelEditSubAction(). So, the direction is related to the top level edit sub action, and we can move it into AutoEditActionDataSetter. Note that except EditSubAction::eDeleteSelectedContent, the relation between sub-action and direction is fixed so that this patch checks the relation with MOZ_ASSERT. If we could replace EditSubAction::eDeleteSelectedContent with information of direction, we'd remove the new member of AutoEditActionDataSetter, though. Differential Revision: https://phabricator.services.mozilla.com/D12400
6c108d9038c3: Bug 1497746 - part 1: Move EditorBase::mTopLevelEditSubAction to EditorBase::AutoEditActionDataSetter r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 26 Nov 2018 03:53:29 +0000 - rev 507164
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1497746 - part 1: Move EditorBase::mTopLevelEditSubAction to EditorBase::AutoEditActionDataSetter r=m_kato EditorBase::mTopLevelEditSubAction is set only by EditorBase::OnStartToHandleTopLevelEditSubAction() and EditorBase::OnEndToHandleTopLevelEditSubAction() and they are called only by AutoTopLevelEditSubActionNotifier(). So, this is used only in stack when a public method of editor is called. Therefore, we can move it into EditorBase::AutoEditActionDataSetter. Then, we can reduce heap allocation for editor instances. Differential Revision: https://phabricator.services.mozilla.com/D12399
3691033d6e78: Bug 1507726 - Update encoding_rs to 0.8.12. r=m_kato
Henri Sivonen <hsivonen@hsivonen.fi> - Thu, 22 Nov 2018 01:41:51 +0000 - rev 506907
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1507726 - Update encoding_rs to 0.8.12. r=m_kato * Improves UTF-8 validation performance. * Improves UTF-8 to UTF-16 decode performance. * Improves non-Latin and Latin1-ish Latin single-byte encode performance. * Improves code quality by addressing some clippy lints. The optional legacy CJK encoder changes are not used by Firefox. Differential Revision: https://phabricator.services.mozilla.com/D12514
824bcd08c85e: Bug 1504911 - part 5: Make HTMLEditor::InsertTableCellsWithTransaction() create AutoPlaceholderBatch and AutoTopLevelEditSubActionNotifier r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 21 Nov 2018 09:30:40 +0000 - rev 506675
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1504911 - part 5: Make HTMLEditor::InsertTableCellsWithTransaction() create AutoPlaceholderBatch and AutoTopLevelEditSubActionNotifier r=m_kato Currently, calling nsITableEditor.insertTableCell() does not cause dispatching "input" event since it does not create AutoPlaceholderBatch. Additionally, different from InsertTableRowsWithTransaction() and InsertTableColumnsWithTransaction(), it does not create AutoTopLevelEditSubActionNotifier. Because of those APIs should work similarly, we should make it creates both auto class instances. Differential Revision: https://phabricator.services.mozilla.com/D12248
7f09364736a0: Bug 1504911 - part 3: Make TextEditRules::WillSetText() not handle anything when EditAction is eReplaceText r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 20 Nov 2018 14:34:32 +0000 - rev 506673
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1504911 - part 3: Make TextEditRules::WillSetText() not handle anything when EditAction is eReplaceText r=m_kato When all editor text is replaced while handling a user operation, editor needs to dispatch "input" event. Therefore, in such case, i.e., EditAction is eReplaceText, TextEditor::SetTextAsSubAction() needs to handle it instead of TextEditRules::WillSetText(). Differential Revision: https://phabricator.services.mozilla.com/D12246
95bd81205750: Bug 1505679 - Make HTMLEditor::RemoveList() sets specific EditAction when it's called by execCommand("insertorderedlist") or execCommand("insertunorderedlist") r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 12 Nov 2018 08:13:58 +0000 - rev 505074
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1505679 - Make HTMLEditor::RemoveList() sets specific EditAction when it's called by execCommand("insertorderedlist") or execCommand("insertunorderedlist") r=m_kato Even when execCommand("insertorderedlist") and execCommand("insertunorderedlist") remove existing lists, we need to set InputEvent.inputType value to "insertOrderedList" or "insertUnorderedList". Fortunately, the XPCOM method is used only for handling execCommand("insertorderedlist") and execCommand("insertunorderedlist") on Firefox. Therefore, we should make it set EditAction to EditAction::eRemoveOrderedListElement or EditAction::RemoveUnorderedListElement. Note that comm-central uses this method directly and uses "cmd_removeList" which causes calling the XPCOM method with empty string. However, input events for them won't be exposed to the web. Therefore, it's okay to set EditAction::eRemoveListElement for the other cases. Differential Revision: https://phabricator.services.mozilla.com/D11439
01cd5b54d3ee: Bug 1504910 - part 3: Make TextEditor::OnDrop() remove selected content before inserting dropped content and fire "input" event before inserting first content r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 12 Nov 2018 07:19:01 +0000 - rev 505073
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1504910 - part 3: Make TextEditor::OnDrop() remove selected content before inserting dropped content and fire "input" event before inserting first content r=m_kato Both Chrome and Safari dispatches 2 sets of "beforeinput" and "input" events when user drag and drop content in same editor. One is "deleteByDrag" and the other is "insertFromDrop". We need to follow same behavior since it should be able to cancel only "deleteByDrag" or "insertFromDrop" when we implement "beforeinput" event. HTMLEditor::InsertObject() may handle asynchronously. And we don't need to do anything additionally for HTMLEditor. Therefore, TextEditor::OnDrop() can delete selection before inserting dropped content by itself. Therefore, this patch makes TextEditor::OnDrop() call TextEditor::PrepareToInsertContent() and EditorBase::FireInputEvent() by itself. Unfortunately, it seems that we cannot write automated tests for this. Even if using synthesizeMouse() of EventUtils, synthesizing mouse events won't generate "dragover" nor "drop" events. Perhaps, like EventUtils.js, we need to do something with drag service, but it means that we cannot emulate drag and drop in an editor. Differential Revision: https://phabricator.services.mozilla.com/D11285
7e18b2160df5: Bug 1504910 - part 2: Make TextEditor::InsertTextAt() and HTMLEditor::DoInsertHTMLWithContext() share preparation code before inserting content r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Mon, 12 Nov 2018 01:40:46 +0000 - rev 505072
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1504910 - part 2: Make TextEditor::InsertTextAt() and HTMLEditor::DoInsertHTMLWithContext() share preparation code before inserting content r=m_kato Only TextEditor::InsertTextAt() and HTMLEditor::DoInsertHTMLWithContext() removes selected content before pasting from clipboard or inserting dropped content, and they do same things. Therefore, they can share the code with a helper method. Note that this makes each root caller won't return NS_ERROR_EDITOR_DESTROYED since the destruction is expected by web app. Differential Revision: https://phabricator.services.mozilla.com/D11284
30487cc4d50c: Bug 1504910 - part 1: Clean up methods which are called by TextEditor::OnDrop() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Fri, 09 Nov 2018 08:40:57 +0000 - rev 505071
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1504910 - part 1: Clean up methods which are called by TextEditor::OnDrop() r=m_kato TextEditor::OnDrop() calls TextEditor::InsertFromDataTransfer() or HTMLEditor::InsertFromDataTransfer() and they are called only by TextEditor::OnDrop(). TextEditor::InsertFromDataTransfer() calls only TextEditor::InsertTextAt() and TextEditor::InsertTextAt() calls only TextEditor::InsertTextAsSubAction() if insertion point is nullptr. Therefore, if the callers sets nullptr, they should call TextEditor::InsertTextAsSubAction() directly. Then, we can make TextEditor::InsertTextAt() require non-nullptr insertion point. HTMLEditor::InsertFromDataTransfer() calls HTMLEditor::InsertObject(), HTMLEditor::DoInsertHTMLWithContext() or TextEditor::InsertTextAt(). HTMLEditor::InsertObject() calls HTMLEditor::DoInsertHTMLWithContext() directly or via BlobReader (in this case, calls asynchronously). Unfortunately both HTMLEditor::InsertObject() and HTMLEditor::DoInsertHTMLWithContext() are called by HTMLEditor::InsertFromTransferable() which is paste event handler. Therefore, we cannot make them require non-nullptr insertion point, though, anyway, they cannot become simpler even if we could do that. This patch marks them as MOZ_CAN_RUN_SCRIPT as far as possible and makes them take |const EditorDOMPoint&| for insertion point. Differential Revision: https://phabricator.services.mozilla.com/D11283
b631af71b7ba: Bug 1503231 - Make TextEditor::DeleteSelectionAsAction() removes removing range information from EditAction when Selection is NOT collapsed r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Wed, 07 Nov 2018 08:38:15 +0000 - rev 504027
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1503231 - Make TextEditor::DeleteSelectionAsAction() removes removing range information from EditAction when Selection is NOT collapsed r=m_kato When Selection is NOT collapsed, we remove selected content. Therefore, web apps don't need to know range information of user operation. However, web apps may want to know direction of the operation (backward or forward). E.g., web apps may just mark selected range as "deleted" and move caret before or after the range. Therefore, when computed EditAction is eDeleteWordBackward or eDeleteToBeginningOfSoftLine, we should use eDeleteBackward instead. When it is eDeleteWordForward or eDeleteToEndOfSoftLine, we should use eDeleteForward instead. Note that only on Windows, we follow behavior of richtext control (and Word). That is, Ctrl + Backspace/Delete collapse from start of selected range to start/end of current word. I.e., collapsing Selection to start first and removing to start or end of current word is Windows's standard behavior. Currently, we do this in DeleteSelectionAsSubAction() but every caller specifies eNone to aDirection except DeleteSelectionAsAction(). So, we can move this before re-computing EditAction in DeleteSelectionAsAction(). Differential Revision: https://phabricator.services.mozilla.com/D10992
7be8263d44fd: Bug 1504131 - part 3: Remove editor/libeditor/HTMLEditorObjectResizerUtils.h r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 06 Nov 2018 06:10:01 +0000 - rev 503809
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1504131 - part 3: Remove editor/libeditor/HTMLEditorObjectResizerUtils.h r=m_kato Now, the header file is not necessary. Differential Revision: https://phabricator.services.mozilla.com/D10870
d0a8f3df072a: Bug 1504131 - part 2: Remove ResizerMouseMotionListener r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 06 Nov 2018 06:09:18 +0000 - rev 503808
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1504131 - part 2: Remove ResizerMouseMotionListener r=m_kato ResizerMouseMotionListener listens to "mousemove" events for resizers or grabber to move absolutely position element and it calls only HTMLEditor::MouseMove(). Fortunately, neither EditorEventListener not HTMLEditorEventListener listens to "mousemove" events. Therefore, we can use HTMLEditorEventListener instead. Differential Revision: https://phabricator.services.mozilla.com/D10869
7160ca524bf0: Bug 1504131 - part 1: Remove DocumentResizeEventListener r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 06 Nov 2018 04:58:29 +0000 - rev 503807
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1504131 - part 1: Remove DocumentResizeEventListener r=m_kato DocumentResizeEventListener listens to only "resize" events of the window and when it fired, it just calls HTMLEditor::RefreshResizers(). Fortunately, neither EditorEventListener nor HTMLEditorEventListener listens to "resize" events. Therefore, we can move this implementation into HTMLEditorEventListener. Differential Revision: https://phabricator.services.mozilla.com/D10866
c470a61009f9: Bug 1504379 - Add automated tests for nsIPlaintextEditor.insertLineBreak() r=m_kato
Masayuki Nakano <masayuki@d-toybox.com> - Tue, 06 Nov 2018 03:47:38 +0000 - rev 503806
Push 1905 by ffxbld-merge at Mon, 21 Jan 2019 12:33:13 +0000
Bug 1504379 - Add automated tests for nsIPlaintextEditor.insertLineBreak() r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D10805