I initially tried to avoid this, but decided it was necessary given the number
of times I had to repeat the same pattern of casting a variable to void*, and
then casting it back in a part of code far distant from the original type.
This changes our preference callback registration functions to match the type
of the callback's closure argument to the actual type of the closure pointer
passed, and then casting it to the type of our generic callback function. This
ensures that the callback function always gets an argument of the type it's
actually expecting without adding any additional runtime memory or
QueryInterface overhead for tracking it.
MozReview-Commit-ID: 9tLKBe10ddP
--HG--
extra : rebase_source : 7524fa8dcd5585f5a31fdeb37d95714f1bb94922
We're already committing jump lists on mIOThread, and I see
no errors or problems with initting on mIOThread either, so
I think this is okay. Had to restructure some things to
make using a Promise simple, and to avoid dispatching to
mIOThread from within mIOThread (since we can only dispatch
to a LazyIdleThread from the owner thread).
MozReview-Commit-ID: 1imBF8Jzmn6
--HG--
extra : rebase_source : 4f912e819cec898910e72d95311e3924714a3090
When user removes all composition string of MS Pinyin, MS Wubi, MS ChangJie and
MS Quick with Backspace key, IME commits last character temporarily and
restart composition to replace the last character with empty string when
user tries to remove last one character.
This causes flicking composition string because the additional composition
selects the character and it may be painted immediately before removed, and
also editor will have unnecessary undo transaction.
Therefore, as same as bug 1208043, TSFTextStore::RecordCompositionStartAction()
should restart last composition in such case. Fortunately, we implemented
similar code for bug 1208043, however, unfortunately, we don't have preceding
pending compositionstart in this case. Therefore, this patch makes
pending compositionend store start offset of composition. Then, we can
restart composition only with information stored by pending compositionend
action. Additionally, this patch renames the method checking pending
actions for self-describing the new meaning.
MozReview-Commit-ID: 1RyuacxEbky
--HG--
extra : rebase_source : 1c8ecc0b63114ae65c77cd76cb85a21d2716442c
Some odd mouse drivers try to activate a window which the mouse driver wants to
scroll its content (such window is typically under the mouse cursor when mouse
wheel is turned). However, this is illegal behavior and such odd mouse drivers
try to activate our popup windows which won't be activated without such apps.
We prevented this odd focus behavior with fixing of bug 953146. However, it
did NOT stop notifying widget listener of activating nor inactivating the
windows. Therefore, that caused a lot of reflow for supporting
-moz-window-inactive pseudo class.
This patch makes nsWindow::DealWithPopups() consume WM_ACTIVATE message before
nsWindow::ProcessMessage() because nsWindow::ProcessMessage() notifies widget
listener of activating and inactivating window even when focus move from and to
our popup window. So, in other words, this patch stops notifying widget
listener of activating and inactivating window when focus moves from/to
a popup window.
MozReview-Commit-ID: 2dyq07zHZKp
--HG--
extra : rebase_source : 8075a3ead73a5f2892a1a1a8e71252e574200bf4
Currently, if an event is consumed in the main process, EventStateManager
does not send it to remote process. However, this is unexpected behavior
for some WidgetKeyboardEvent dispatchers. OS sometimes has consumed native
key events before sending applications. For example, Alt key on Windows
should activate menu bar of focused window but Alt key may be consumed before
focused window receives the event. In such case, we mark Alt keyboard event
as "consumed before dispatch", and chrome treat it like as its preventDefault()
is called in web content. (Note that for compatibility with other browsers,
the consumed state is not exposed to web content. So, Event.defaultPrevented
returns false in web content.)
Therefore, we need to treat "consumed" state and "cross process forwarding"
state separately. This patch makes calling WidgetEvent::PreventDefault()
always stops cross process forwarding for backward compatibility. Additionally,
for the special case mentioned above, this patch makes
WidgetEvent::PreventDefaultBeforeDispatch() take additional argument,
|aIfStopCrossProcessForwarding|. If this is CrossProcessForwarding::eStop,
the event won't be sent to remote process as same as calling PreventDefault().
Otherwise, CrossProcessForwarding::eHold, PreventDefaultBeforeDispatch() call
does not change "cross process forwarding" state. I.e., if the event's
StopCrossProcessForwarding() and PreventDefault() are not called until
EventStateManager::PostHandleEvent(), the event will be sent to remote process
as usual.
MozReview-Commit-ID: IQGWJvXetxV
--HG--
extra : rebase_source : 4ccdd500e80b8fe29e469ac3b85578e1c07c8358
We always struggle with a lot of IME bugs on Windows. Currently, any IME
vendors should've already released TIP for TSF rather than legacy IMM-IME
since IMM-IME is not available on UWP apps. Additionally, due to API
limitation, it's difficult to get human-friendly name of IMM-IME. So, let's
collect only TIP names of TSF on Windows. This must be enough.
Note that we cannot get common-English name even though the API to retrieve
TIP name taking language code. Therefore, a TIP may be collected with
different name, e.g., one is Japanese name and the other is English name.
If we collect GUIDs of TIP, we can avoid this issue. However, it's
difficult to collect both GUID and human-friendly name since Telemetry
key is up to 72 characters.
Currently, I give up to avoid this duplicated issue. Perhaps, this is not
so serious issue since most TIP users must match language of TIP and their
system language settings. Therefore, this patch collects Locale ID of
TIP and description of it. Locale ID is necessary because some TIPs may be
named same name for different languages. For example, both Japanese and
Hangul IMEs of Microsoft are named as "Microsoft IME".
MozReview-Commit-ID: IeSxfeqS62a
--HG--
extra : rebase_source : b269ce3c41f7a998193972afb31183a61d3948be
This patch is an automatic replacement of s/NS_NOTREACHED/MOZ_ASSERT_UNREACHABLE/. Reindenting long lines and whitespace fixups follow in patch 6b.
MozReview-Commit-ID: 5UQVHElSpCr
--HG--
extra : rebase_source : 4c1b2fc32b269342f07639266b64941e2270e9c4
extra : source : 907543f6eae716f23a6de52b1ffb1c82908d158a
When AltGr key is pressed, following messages come:
1. WM_KEYDOWN for ControlLeft
2. WM_KEYDOWN for AltLeft
3. WM_SYSKEYUP for ControlLeft
4. WM_KEYUP for AltLeft
In these key sequence, KeyboardEvent.key value of keydown event at #2 and keyup
event at #4 should be "AltGraph". This patch fixes the key value and
adding new test into test_keycodes.xul to check the behavior with
SynthesizeNativeKey().
MozReview-Commit-ID: JZ6WednB8la
--HG--
extra : rebase_source : 596371ede89e90c23f7e842b26ec8155b911fe60
Users can emulate AltGr key with pressing both Ctrl key and Alt key on Windows
since AltGr is represented as so in Windows and physical keyboard may not have
AltRight key.
If user emulates AltGr key, we should set MODIFIER_ALTGRAPH to a set of
keyboard events for printable keys only when the key press produces
character(s) or a dead key. For example:
1. ControlLeft keydown event should make ctrlKey true.
2. AltLeft keydown event should make altKey true (not AltGraph state).
3. ctrlKey and altKey of printable keydown, keypress and keyup events should be
set to false, but getModifierState("AltGraph") should return true.
4. AltLeft keyup event should make altKey false.
5. ControlLeft keyup event should make ctrlKey false.
(If AltLeft key is pressed first, altKey of AltLeft keydown is true and
both altKey and ctrlKey of the following ControlLeft keydown are true as
usual.)
MozReview-Commit-ID: 8Km8GXPDQw1
--HG--
extra : rebase_source : f4924f075c68361c8ce563910280ea24774c519f
KeyboardLayout::InitNativeKey() takes |const ModifierKeyState&| as its
argument with NativeKey reference and it calls some internal methods with
the given ModifierKeyState without any changes. Additionally, its caller
is only NativeKey::InitWithKeyChar() and its called with given NativeKey
instance's mModKeyState. So, removing the redundant arguments from
some methods makes them clearer what they compute with.
So, this patch does not change any behavior.
MozReview-Commit-ID: 3w9Ee7PMU05
--HG--
extra : rebase_source : b724a18d5a14672e60ffa5fb9feca5c11dac42a3
By the proposal from Google, <https://github.com/w3c/uievents/issues/147>,
Chromium treat AltRight key as "AltGraph" modifier if the keyboard layout
has AltGr key.
When AltRight key is pressed with a keyboard layout which has AltGr key,
modifiers should as following:
1. "keydown" for ControlLeft:
ctrlKey: true, altKey: false, getModifierState("AltGraph"): false
2. "keydown" for AltRight:
ctrlKey: false, altKey: false, getModifierState("AltGraph"): true
3. Some "keydown", "keypress" and "keyup" events:
ctrlKey: false, altKey: false, getModifierState("AltGraph"): true
4. "keyup" for ControlLeft:
ctrlKey: false, altKey: false, getModifierState("AltGraph"): true
5. "keyup" for AltRight:
ctrlKey: false, altKey: false, getModifierState("AltGraph"): false
So, only when the preceding "keydown" event for ControlLeft, ctrlKey should
be set to true as usual. However, after AltRight key is pressed actually,
we should treat "AltGraph" modifier is true and both ctrlKey and altKey
should be set to false for web apps can handle text input normally.
So, MODIFIER_ALTGRAPH and MODIFIER_CONTROL/MODIFIER_ALT should not be set
at the same time.
This patch makes ModifierKeyState have only MODIFIER_ALTGRAPH or
MODIFIER_CONTROL/MODIFIER_ALT.
Additionally, this patch makes VirtualKey::ShiftState treat "AltGraph" as a
modifier. So, now, VirtualKey needs to convert ShiftState to index value when
it accesses its mShiftStates array. Therefore, this patch adds
VirtualKey::ToIndex() and make each VirtualKey method use it before
accessing mShiftStates.
Note that this patch also fixes bug of WinUtils::SetupKeyModifiersSequence().
The constructor of KeyPair takes 2 keycode values, but the second virtual
keycode can have scancode to distinguish if the key is left or right.
However, WinUtils::SetupKeyModifiersSequence() never sets scancode to
KeyPair. Therefore, it fails to dispatch AltRight key event.
MozReview-Commit-ID: 7ealxJH9KlZ
--HG--
extra : rebase_source : 761bc4416222def020a0731d6ae7940ef074ebe0
For setting AltRight key's key value to "AltGraph" if it should work as so,
we need to know if current keyboard layout has AltGr key. Unfortunately,
Windows doesn't provide such information but we retrieve all input characters
from each key when a keyboard layout is loaded. So, when we load a keyboard
layout, we can mark if current keyboard layout has AltGr key with checking
at least one key inputs different character(s) when AltGr key is pressed.
MozReview-Commit-ID: 8GI3phSVTUS
--HG--
extra : rebase_source : f1622615f03740609984da6d216391e23cae6796
Currently, TSFTextStore::GetTextExt() won't return TS_E_NOLAYOUT error when
ATOK retrieves text rect of all of the composition string.
However, if user converts 2nd or later clause, ATOK retrieves text rect after
start of the character of selected clause. Returning TS_E_NOLAYOUT in this
case causes candidate window being positioned temporarily below first character
of the composition string.
For avoiding the flicker of the candidate window, TSFTextStore::GetTextExt()
shouldn't return TS_E_NOLAYOUT when ATOK retrieves text rects *in* the
composition string.
MozReview-Commit-ID: Cp17HmP2QGK
--HG--
extra : rebase_source : bdd2d2867cccdcf1fe39612ca2f52872d472cb7a
Old ATOK referred native caret position to decide its candidate window position.
However, at least ATOK 2016 does not need to refer it. Additionally, if we
create native caret for ATOK 2016, the candidate window position, ATOK 2016
refers the native caret only when we cannot return expected rect. Therefore,
only immediately after modifying composition string, the position is different
from actual position by a couple of pixels and that looks like flicks the
candidate window.
So, we should stop creating native caret for ATOK 2016 (as same as ATOK 2017).
MozReview-Commit-ID: LsmVXCmRIzc
--HG--
extra : rebase_source : 30b7d15cb23e567b14e1231909aa5a17bdf909a6
Fairly straightforward, just a blanket removal. Haven't heard
anything on dev-platform or fx-data-dev regarding this removal,
so I think it's likely safe to remove on Nightly, and we can
revert if anyone makes a fuss.
As part of removing the HangMonitor, I renamed a few things and
reorganized the namespaces to not depend on a HangMonitor
namespace. Hopefully this doesn't produce too much noise in the
diff, it just seemed appropriate to move everything around
rather than keep dangling vestiges of the old system.
MozReview-Commit-ID: 8C8NFnOP5GU
--HG--
extra : rebase_source : dd000a05bfc2da40c586644d33ca4508fa5330f6
Currently, TabChild discards eKeyDown and eKeyPress events which are marked as
"repeated" and were dispatched after the latest eKeyDown event comes into the
process. However, keyboard layout utils may generate native key events
as "repeated" even if each native key is important to input proper text.
So, TabChild shouldn't decide if coming keyboard event is skippable only with
mIsRepeat. For solving this issue, this patch adds
mMaybeSkippableInRemoteProcess to WidgetKeyboardEvent and makes
TabChild::SkipRepeatedKeyEvent() check
WidgetKeyboardEvent::CanSkipInRemoteProcess() instead.
On Windows, there are two ways to generate keyboard input messages. One is
using SendMessage() or PostMessage(). The other is SendInput() API. In both
ways, utils can make their input as repeated key messages.
The former case must be safe for this issue since such utils need to set 31st
bit of lParam to 1 explicitly.
On the other hand, in the latter case, the utils probably need to append
KEYEVENTF_KEYUP into KEYBDINPUT::dwFlags. Otherwise, only first call is
treated as non-repeated event.
So, when given message does not came from physical key operation, NativeKey
should set WidgetKeyboardEvent::mMaybeSkippableInRemoteProcess to false
even if WidgetKeyboardEvent::mIsRepeat is true.
MozReview-Commit-ID: 3rinrOjx8Tf
--HG--
extra : rebase_source : 26b6d869260176fc7ef535323b83001bb4b725c2
Fairly straightforward, just a blanket removal. Haven't heard
anything on dev-platform or fx-data-dev regarding this removal,
so I think it's likely safe to remove on Nightly, and we can
revert if anyone makes a fuss.
As part of removing the HangMonitor, I renamed a few things and
reorganized the namespaces to not depend on a HangMonitor
namespace. Hopefully this doesn't produce too much noise in the
diff, it just seemed appropriate to move everything around
rather than keep dangling vestiges of the old system.
MozReview-Commit-ID: 8C8NFnOP5GU
--HG--
extra : rebase_source : 59e4a6ced7d14d2a01c0b79e944078ea84cae523
Refactored StyleComplexColor to support "complex" blending between
background (numeric) color and foreground color (currentColor).
Made explicit the distinction between numeric, currentColor and a
complex blend in Gecko and Stylo.
This is to support SMIL animation, for example, of the form:
<animate from="rgb(10,20,30)" by="currentColor" ... />
MozReview-Commit-ID: IUAK8P07gtm
--HG--
extra : rebase_source : d3648101c6f65479b21e6f02945731cd5bb57663
When runnables are posted to the main thread's event loop, the event
loop notifies any thread observers that this has been done. The app
shell registers itself as just such a runnable, and posts messages to
the native event loop that processing native events needs to be done.
On Windows, this posting takes an extra reference to the app shell, to
keep it alive until the message is processed by the native event loop,
since app shell code needs to be invoked during that processing. The
processing releases this extra reference, so everything stays balanced.
Except that it's possible for messages to be posted to the native event
loop, and then browser shutdown happens. Those messages are not
processed and the associated references taken are not released. This
imbalance means that in debug builds, we appear to be leaking the app
shell, and that leaking results in intermittent oranges.
This intermittent orange has manifested itself in a variety of ways over
the years, depending on how big the app shell itself was (since that
changes the number of bytes leaked) and how many leak-checked things the
app shell was holding on to. This bug is merely the latest
manifestation; the last serious work on analyzing the phenomenon and
fixing it was done in bug 1220517.
The solution proposed in that bug was that we simply stop the extra
reference counting; when the app shell is destroyed normally, we
shouldn't be processing the native event loop any more anyway. So even
if the native event loop is holding (freed) pointers to the app shell,
we'd never execute the callback and perform a use-after-free. Reading
through the code suggests that this *ought* to be the case, but the
potential for shooting ourselves in the foot seems awfully high.
In any event, this is not a problem unique to Windows; we have seen this
same sort of thing happen on OS X. See nsAppShell::ProcessGeckoEvents
in widget/cocoa/nsAppShell.mm.
Here we propose a slightly different solution: we keep track of the
number of native event callbacks we have scheduled, incrementing when we
schedule, and decrementing when we actually run one. When the app shell
is destroyed, we simply set the number of outstanding events to zero,
and we prohibit the callback from accessing the app shell if there are
no outstanding events. This solution is analogous to dropping the extra
reference counting, but avoids potential badness if we do wind up
processing native events after the app shell is destroyed.
This was done automatically replacing:
s/mozilla::Move/std::move/
s/ Move(/ std::move(/
s/(Move(/(std::move(/
Removing the 'using mozilla::Move;' lines.
And then with a few manual fixups, see the bug for the split series..
MozReview-Commit-ID: Jxze3adipUh
Fairly straightforward, just a blanket removal. Haven't heard
anything on dev-platform or fx-data-dev regarding this removal,
so I think it's likely safe to remove on Nightly, and we can
revert if anyone makes a fuss.
As part of removing the HangMonitor, I renamed a few things and
reorganized the namespaces to not depend on a HangMonitor
namespace. Hopefully this doesn't produce too much noise in the
diff, it just seemed appropriate to move everything around
rather than keep dangling vestiges of the old system.
MozReview-Commit-ID: 8C8NFnOP5GU
--HG--
extra : rebase_source : a8840bd26f4b01b756ffa72345ababb625048550