840409 - Implement IMEHandler which hides nsIMM32Handler and nsTextStore from non-IME handlers

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

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

Currently, there are two IME handlers nsIMM32Handler and nsTextStore. nsWindow and nsAppShell now access them directly.

However, it makes the caller's code messy. We should add widget::IMEHandler between them and non-IME handlers.

Then, we can prevent that somebody forgets to call one of them. For example, nsWindow calls nsIMM32Handler::IsComposingOn(nsWindow*) from a couple of places, however, they don't check nsTextStore's state. This is obviously a bug.

We should correct such things.

Comment 1

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

Attachment: part.1 Implement widget::IMEHandler with Initialize() and Terminate() methods

First, create widget::IMEHandler for a mediator class for non-IME handlers. Non-IME handler classes shouldn't need to worry the difference between nsIMM32Handler and nsTextStore.

This patch also implements Initialize() and Terminate(). They are called only once per process at starting or ending it. These methods call both nsIMM32Handler's method and nsTextStore's method intentionally. Especially, nsIMM32Handler::Initialize() is necessary for initializing the IMM's log module. If the methods of nsIMM32Handler is accidentally called in TSF mode, it causes crash if we don't call it.

Comment 2

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

Attachment: part.2 Move nsIMM32Handler::IsDoingKakuteiUndo() to widget::IMEContext

This patch moves nsIMM32Handler::IsDoingKakuteiUndo() to widget::IMEHandler. The method just checks the message queue. So, it doesn't depend on whether it's in TSF mode or IMM mode.

Comment 3

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

Attachment: part.3 Implement widget::IMEHandler::ProcessMessage()

This and later patches separate the mode. This IMEHandler::ProcessMessage() calls nsIMM32Handler::ProcessMessage() only when it's not in TSF mode.

Note that we should nsTextStore::ProcessMessage() in the future. In TSF mode and there is composition, nsTextStore should handle some messages directly, e.g., WM_KEYDOWN.

Comment 4

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

Attachment: part.4 Implement widget::IMEHandler::CurrentKeyboardLayoutHasIME() for debug

This patch implements widget::IMEHandler::CurrentKeyboardLayoutHasIME().

I realized that ::ImmIsIME() always returns true starting Vista. So, current code has a bug. However, it's not problem because the method is only used for NS_ASSERTION() and MOZ_ASSERT() (i.e., used only in debug build).

The new method, nsTextStore::CurrentKeyboardLayoutHasIME(), returns expected value.

Comment 5

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

Attachment: part.5 Implement widget::IMEHandler::IsComposing() and widget::IMEHandler::IsComposingOn()

Current code checks only the state in nsIMM32Handler. This is obviously a bug!

Comment 6

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

Attachment: part.6 Implement widget::IMEHandler::NotifyIME() for committing and canceling composition

I have a plan to union some IME methods of nsIWidget to nsIWidget::NotifyIME(NotificationToIME aNotification) in bug 558976. This patch is also preparing that on Windows.

Roc, if you have some idea for better name and better argument style, let me know. See also following patch which add new notification.

Comment 7

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

Attachment: part.7 Define new notification for IME of focus or selection change and handle them in widget::IMEHandler::NotifyIME()

This adds new notifications. FOCUS and BLUR doesn't have any problem, I think. SELECTION_CHANGE has an issue. It's similar to CURSOR_POS_CHANGED. Why I separate them is that SELECTION_CHANGE will be used for nsIWidget::OnIMESelectionChange() which is called by nsIMEStateManager. However, CURSOR_POS_CHANGED is used by nsEditor for calling reset_input_state API on GTK. So, they are really different for now (in other words, CURSOR_POS_CHANGED and REQUEST_TO_COMMIT are same for now). In the future, nsEditor shouldn't use CURSOR_POS_CHANGED. It should be handled by nsIMEStateManager for making IME code simpler. At least for now, I think that we should define these 3 new values for nsIWidget::OnIMEFocusChange(true), nsIWidget::OnIMEFocusChange(false) and nsIWidget::OnIMESelectionChange().

Unfortunately, I have no idea to union nsIWidget::OnIMETextChange() because it has 3 uint32_t arguments. If nsIWidget::NotifiyIME() takes a void* argument for additional information, we can merge them, but it doesn't look smart. So, I give up to merge OnIMETextChange() now.

Comment 8

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

Attachment: part.8 Implement widget::IMEHandler::NotifyIMEOfTextChange()

This just implements widget::IMEHandler::NotifyIMEOfTextChange() for nsIWidget::OnIMETextChange().

Comment 9

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

Attachment: part.9 Implement widget::IMEHandler::GetUpdatePreference()

Just implements widget::IMEHandler::GetUpdatePreference().

Comment 10

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

Attachment: part.10 Implement widget::IMEHandler::SetOpenState() and widget::IMEHandler::GetOpenState()

nsWindow should use only proper module's SetOpenState() and GetOpenState().

Comment 11

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

Attachment: part.11 Implement nsIMEContext::AssociateDefaultContext() and nsIMEContext::Disassociate()

Handling IME context is really a part of IMM handler. We should move associating/disassociating IMC code from nsWindow to nsIMEContext which is in nsIMM32Handler.h.

Comment 12

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

Attachment: part.12 Implement widget::IMEHandler::InitInputContext() and widget::IMEHandler::SetInputContext()

This patch moves the code in nsWindow::SetInputContext() to widget::IMEHandler::SetInputContext(). Unfortunately, nsTextStore doesn't handle IME state when any editable elements don't have focus. Therefore, we still need to use nsIMEContext class even in TSF mode in the IMEHandler::SetInputContext().

We should fix this issue ASAP, but I guess that the change isn't small. So, we should do it later.

Comment 13

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

Attachment: part.13 Implement widget::IMEHandler::IsIMEEnabled() which checks whether the state indicates IME available or not

widget::IMEHandler::IsIMEEnabled() returns whether the IMEStatus::ENABLED indicates IME being available or not. By this change, we can hide nsIMM32Handler from nsWindow completely.

Comment 14

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

Attachment: part.14 Implement widget::IMEHandler::GetNativeData()

GetNativeData() caller attempts to retrieve something for IME related resource, it should be done via IMEHandler. Then, we can hide nsTextStore from nsWindow completely.

Comment 15

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

Attachment: part.15 Implement widget::IMEHandler::CanOptimizeKeyAndIMEMessages()

widget::IMEHandler::CanOptimizeKeyAndIMEMessages() hides nsIMM32Handler from nsAppShell. We need to implement it for nsTextStore too, though.

I realized that WM_KEYDOWN message never comes in TSF mode if IME eats the message. So, the method doesn't work in TSF mode anyway. I need to research it later.

Comment 16

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

I realized that when IME state is PLUGIN, nsTextStore doesn't handle IME. However, the window should be associated the default IMC for plugin being able to handle IMM and we need to call nsIMM32Handler for dispatching plugin events which send native IME events to plugin. I'll make followup patch (maybe, only part.16).

Comment 17

[Jim Mathies [:jimm]]

Have you built these with --enable-metro, or thrown them at try? Looks like your missing the interfacing to MetroWidget.

Comment 18

[Jim Mathies [:jimm]]

(In reply to Jim Mathies [:jimm] from comment #17)
> Have you built these with --enable-metro, or thrown them at try? Looks like
> your missing the interfacing to MetroWidget.

Maybe I jumped to a bad conclusion, I do see metro widget interface code in at least one patch.

Comment 19

[Jim Mathies [:jimm]]

Comment on attachment 715347 [details] [diff] [review]
part.1 Implement widget::IMEHandler with Initialize() and Terminate() methods

Review of attachment 715347 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/WinIMEHandler.cpp
@@ +29,5 @@
> +  nsTextStore::Initialize();
> +  sIsInTSFMode = nsTextStore::IsInTSFMode();
> +#endif // #ifdef NS_ENABLE_TSF
> +
> +  nsIMM32Handler::Initialize();

Can we skip initializing IMM32 on platforms that don't support it?

Comment 20

[Jim Mathies [:jimm]]

I'll keep working through these today. For testing purposes I'd really appreciate a rollup patch merged to mc tip.

Comment 21

[Jim Mathies [:jimm]]

Comment on attachment 715353 [details] [diff] [review]
part.4 Implement widget::IMEHandler::CurrentKeyboardLayoutHasIME() for debug

Review of attachment 715353 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/nsTextStore.cpp
@@ +3213,5 @@
> +         "ITfInputProcessorProfileMgr"));
> +      return false;
> +    }
> +    // If the profiles instance doesn't have ITfInputProcessorProfileMgr
> +    // interface, that means probably we're running on WinXP or Win2k3 (not R2).

nit, we don't support win2k anymore. Win XP SP2 and up.

Comment 22

[Jim Mathies [:jimm]]

Comment on attachment 715355 [details] [diff] [review]
part.5 Implement widget::IMEHandler::IsComposing() and widget::IMEHandler::IsComposingOn()

Review of attachment 715355 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/WinIMEHandler.cpp
@@ +84,5 @@
> +}
> +
> +// static
> +bool
> +IMEHandler::IsComposingOn(nsWindow* aWindow)

Shouldn't all outward interfaces be nsIWidget pointers here? We're not using this in metro, but I know we had to make these changes to the text store module.

Comment 23

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

(In reply to Jim Mathies [:jimm] from comment #19)
> Comment on attachment 715347 [details] [diff] [review]
> part.1 Implement widget::IMEHandler with Initialize() and Terminate() methods
> 
> Review of attachment 715347 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/windows/WinIMEHandler.cpp
> @@ +29,5 @@
> > +  nsTextStore::Initialize();
> > +  sIsInTSFMode = nsTextStore::IsInTSFMode();
> > +#endif // #ifdef NS_ENABLE_TSF
> > +
> > +  nsIMM32Handler::Initialize();
> 
> Can we skip initializing IMM32 on platforms that don't support it?

See comment 16. No. When the enabled state is PLUGIN (i.e., plugin has focus), we need to fallback to IMM. nsIMM32Handler dispatches IMM events to the plugin. Although, it might be able to be moved to widget::IMEHandler.

(In reply to Jim Mathies [:jimm] from comment #18)
> (In reply to Jim Mathies [:jimm] from comment #17)
> > Have you built these with --enable-metro, or thrown them at try? Looks like
> > your missing the interfacing to MetroWidget.
> 
> Maybe I jumped to a bad conclusion, I do see metro widget interface code in
> at least one patch.

Yes. Metro widget basically doesn't need any change about this bug because Metro widget ONLY uses nsTextStore. So, it doesn't need to worry/check the running IME mode like desktop.

(In reply to Jim Mathies [:jimm] from comment #21)
> Comment on attachment 715353 [details] [diff] [review]
> part.4 Implement widget::IMEHandler::CurrentKeyboardLayoutHasIME() for debug
> 
> Review of attachment 715353 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/windows/nsTextStore.cpp
> @@ +3213,5 @@
> > +         "ITfInputProcessorProfileMgr"));
> > +      return false;
> > +    }
> > +    // If the profiles instance doesn't have ITfInputProcessorProfileMgr
> > +    // interface, that means probably we're running on WinXP or Win2k3 (not R2).
> 
> nit, we don't support win2k anymore. Win XP SP2 and up.

Yep. The interface is available Win2k3 R2 or Win Vista or later. On WinXP or Win2k3, it's not available. See "Requirements" section of following document:
http://msdn.microsoft.com/en-us/library/windows/desktop/aa381941%28v=vs.85%29.aspx

(In reply to Jim Mathies [:jimm] from comment #22)
> Comment on attachment 715355 [details] [diff] [review]
> part.5 Implement widget::IMEHandler::IsComposing() and
> widget::IMEHandler::IsComposingOn()
> 
> Review of attachment 715355 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/windows/WinIMEHandler.cpp
> @@ +84,5 @@
> > +}
> > +
> > +// static
> > +bool
> > +IMEHandler::IsComposingOn(nsWindow* aWindow)
> 
> Shouldn't all outward interfaces be nsIWidget pointers here? We're not using
> this in metro, but I know we had to make these changes to the text store
> module.

I don't think so. As I mentioned above, IMEHandler is designed only for desktop. So, we don't need to use super class here. Although, nsTextStore must not use nsWindow, of course.

Comment 24

[Jim Mathies [:jimm]]

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #23)
> (In reply to Jim Mathies [:jimm] from comment #19)
> > nit, we don't support win2k anymore. Win XP SP2 and up.
> 
> Yep. The interface is available Win2k3 R2 or Win Vista or later. On WinXP or
> Win2k3, it's not available. See "Requirements" section of following document:
> http://msdn.microsoft.com/en-us/library/windows/desktop/aa381941%28v=vs.
> 85%29.aspx

Yes but our code doesn't support win2k. I was just nit'ing your comment that mentions win2k support.

Comment 25

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

(In reply to Jim Mathies [:jimm] from comment #24)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #23)
> > (In reply to Jim Mathies [:jimm] from comment #19)
> > > nit, we don't support win2k anymore. Win XP SP2 and up.
> > 
> > Yep. The interface is available Win2k3 R2 or Win Vista or later. On WinXP or
> > Win2k3, it's not available. See "Requirements" section of following document:
> > http://msdn.microsoft.com/en-us/library/windows/desktop/aa381941%28v=vs.
> > 85%29.aspx
> 
> Yes but our code doesn't support win2k. I was just nit'ing your comment that
> mentions win2k support.

Ah, I wrote "2k3", not "2k". I'll rewrite it as "2003".

Comment 26

[Jim Mathies [:jimm]]

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #25)
> (In reply to Jim Mathies [:jimm] from comment #24)
> > (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #23)
> > > (In reply to Jim Mathies [:jimm] from comment #19)
> > > > nit, we don't support win2k anymore. Win XP SP2 and up.
> > > 
> > > Yep. The interface is available Win2k3 R2 or Win Vista or later. On WinXP or
> > > Win2k3, it's not available. See "Requirements" section of following document:
> > > http://msdn.microsoft.com/en-us/library/windows/desktop/aa381941%28v=vs.
> > > 85%29.aspx
> > 
> > Yes but our code doesn't support win2k. I was just nit'ing your comment that
> > mentions win2k support.
> 
> Ah, I wrote "2k3", not "2k". I'll rewrite it as "2003".

Do we support any flavor of 2k? I was under the impression we dropped all 2k support.

Comment 27

[Masatoshi Kimura [:emk]]

(In reply to Jim Mathies [:jimm] from comment #26)
> Do we support any flavor of 2k? I was under the impression we dropped all 2k
> support.

We do not support "2k"(=2000), but support "2k3"(=2003).
ITfInputProcessorProfileMgr is only available >= "2k3" (NOT "2k"). So we have to consider about unsupported platforms (WinXP and Win2003 SP0/1).

Comment 28

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

Yeah, IIRC, 2k3 is NT5.2 which is greater than NT5.1 (XP).

Comment 29

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

Comment on attachment 715373 [details] [diff] [review]
part.11 Implement nsIMEContext::AssociateDefaultContext() and nsIMEContext::Disassociate()

Oops, at OnDestroyWindow(), we need to restore the default context.

Comment 30

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

Attachment: part.11 Implement nsIMEContext::AssociateDefaultContext() and nsIMEContext::Disassociate()

Comment 31

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

Attachment: part.12 Implement widget::IMEHandler::InitInputContext() and widget::IMEHandler::SetInputContext()

Comment 32

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

Attachment: part.16 Use IMM even in TSF mode when plugin has focus

This patch uses IMM path when a plugin has focus. This is actually works on WinXP with Flash Player without OOPP.

Comment 33

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

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=6ed5d0786832

Comment 34

[Jim Mathies [:jimm]]

Comment on attachment 715381 [details] [diff] [review]
part.13 Implement widget::IMEHandler::IsIMEEnabled() which checks whether the state indicates IME available or not

Review of attachment 715381 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/nsWindow.cpp
@@ +7388,5 @@
>  nsWindow::GetInputContext()
>  {
>    mInputContext.mIMEState.mOpen = IMEState::CLOSED;
> +  if (IMEHandler::IsIMEEnabled(mInputContext) && IMEHandler::GetOpenState(this)) {
> +    mInputContext.mIMEState.mOpen = IMEState::OPEN;

Seems odd that we access a member variable here. Maybe implement a helper?

Comment 35

[Jim Mathies [:jimm]]

Comment on attachment 715386 [details] [diff] [review]
part.15 Implement widget::IMEHandler::CanOptimizeKeyAndIMEMessages()

Review of attachment 715386 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/nsTextStore.h
@@ +147,5 @@
>  
> +  static bool CanOptimizeKeyAndIMEMessages()
> +  {
> +    // TODO: We need to implement this for ATOK.
> +    return true;

This is going to be false for metro.

Comment 36

[Jim Mathies [:jimm]]

Comment on attachment 716406 [details] [diff] [review]
part.16 Use IMM even in TSF mode when plugin has focus

Review of attachment 716406 [details] [diff] [review]:
-----------------------------------------------------------------

haazah!

Comment 37

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

(In reply to Jim Mathies [:jimm] from comment #34)
> Comment on attachment 715381 [details] [diff] [review]
> part.13 Implement widget::IMEHandler::IsIMEEnabled() which checks whether
> the state indicates IME available or not
> 
> Review of attachment 715381 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/windows/nsWindow.cpp
> @@ +7388,5 @@
> >  nsWindow::GetInputContext()
> >  {
> >    mInputContext.mIMEState.mOpen = IMEState::CLOSED;
> > +  if (IMEHandler::IsIMEEnabled(mInputContext) && IMEHandler::GetOpenState(this)) {
> > +    mInputContext.mIMEState.mOpen = IMEState::OPEN;
> 
> Seems odd that we access a member variable here. Maybe implement a helper?

All of mInputContext are initialized at SetInputContext() except the open state. So, in GetInputContext(), only mOpen needs to be initialized. When we returns 'open', IME has to be enabled. Therefore, it checks the mInputContext's enabled state.

(In reply to Jim Mathies [:jimm] from comment #35)
> Comment on attachment 715386 [details] [diff] [review]
> part.15 Implement widget::IMEHandler::CanOptimizeKeyAndIMEMessages()
> 
> Review of attachment 715386 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/windows/nsTextStore.h
> @@ +147,5 @@
> >  
> > +  static bool CanOptimizeKeyAndIMEMessages()
> > +  {
> > +    // TODO: We need to implement this for ATOK.
> > +    return true;
> 
> This is going to be false for metro.

It's great. I hate the message order optimization since it causes some hack :-(

Comment 38

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

Thank you for your quick review.

Comment 39

[Jim Mathies [:jimm]]

> > > +  static bool CanOptimizeKeyAndIMEMessages()
> > > +  {
> > > +    // TODO: We need to implement this for ATOK.
> > > +    return true;
> > 
> > This is going to be false for metro.
> 
> It's great. I hate the message order optimization since it causes some hack
> :-(

Maybe we should file a bug to investigate removing it for desktop? I think the motivation for doing this (perf on older systems) may no longer exist.

We tried removing this one - 

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsAppShell.cpp&branch=3.68&root=/cvsroot&subdir=mozilla/widget/src/windows&command=DIFF_FRAMESET&rev1=3.29&rev2=3.30

and backed it out due to problems with incremental layout.

https://bugzilla.mozilla.org/show_bug.cgi?id=36849

Comment 40

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/990ce017ec91
https://hg.mozilla.org/integration/mozilla-inbound/rev/0de41d77f147
https://hg.mozilla.org/integration/mozilla-inbound/rev/104f94cc779b
https://hg.mozilla.org/integration/mozilla-inbound/rev/beb3e3ab5177
https://hg.mozilla.org/integration/mozilla-inbound/rev/bdc309a7e3e2
https://hg.mozilla.org/integration/mozilla-inbound/rev/41a74153cc88
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a50a2b48d03
https://hg.mozilla.org/integration/mozilla-inbound/rev/59038ecbd55b
https://hg.mozilla.org/integration/mozilla-inbound/rev/398295b6d0e4
https://hg.mozilla.org/integration/mozilla-inbound/rev/46013161c928
https://hg.mozilla.org/integration/mozilla-inbound/rev/9b7c0dfbd87a
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b6be794fbb0
https://hg.mozilla.org/integration/mozilla-inbound/rev/94813538595c
https://hg.mozilla.org/integration/mozilla-inbound/rev/5fe283c4cd7f
https://hg.mozilla.org/integration/mozilla-inbound/rev/f98e63db4452
https://hg.mozilla.org/integration/mozilla-inbound/rev/03e5c19f88ca

Comment 41

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/990ce017ec91
https://hg.mozilla.org/mozilla-central/rev/0de41d77f147
https://hg.mozilla.org/mozilla-central/rev/104f94cc779b
https://hg.mozilla.org/mozilla-central/rev/beb3e3ab5177
https://hg.mozilla.org/mozilla-central/rev/bdc309a7e3e2
https://hg.mozilla.org/mozilla-central/rev/41a74153cc88
https://hg.mozilla.org/mozilla-central/rev/7a50a2b48d03
https://hg.mozilla.org/mozilla-central/rev/59038ecbd55b
https://hg.mozilla.org/mozilla-central/rev/398295b6d0e4
https://hg.mozilla.org/mozilla-central/rev/46013161c928
https://hg.mozilla.org/mozilla-central/rev/9b7c0dfbd87a
https://hg.mozilla.org/mozilla-central/rev/4b6be794fbb0
https://hg.mozilla.org/mozilla-central/rev/94813538595c
https://hg.mozilla.org/mozilla-central/rev/5fe283c4cd7f
https://hg.mozilla.org/mozilla-central/rev/f98e63db4452
https://hg.mozilla.org/mozilla-central/rev/03e5c19f88ca