Closed Bug 1404206 Opened 2 years ago Closed 2 years ago

Needs discussion about inputmode value of Smart Location Bar

Categories

(Firefox :: Address Bar, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 58
Tracking Status
firefox57 + fixed
firefox58 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: inputmethod, Whiteboard: [fxsearch][triage])

Attachments

(3 files, 3 obsolete files)

3.29 KB, patch
Details | Diff | Splinter Review
12.00 KB, patch
Details | Diff | Splinter Review
2.09 KB, patch
Details | Diff | Splinter Review
Starting from bug 1240208, the Smart Location Bar has inputmode as "url":
https://searchfox.org/mozilla-central/rev/20e41d4a61a8f5e34c9cf357304b78b3e9bced8a/browser/base/content/urlbarBindings.xml#21,26,28,39,42

However, this causes bad UX with MS-IME for Japanese and Google Japanese Input on Windows since they automatically set IME open state to "closed" when input scope is only "URL". One possible solution is creating a pref to disable to set input scope in this case (bug 1253165).

On the other hand, now, default Photon UI does NOT have search bar with new profile. That means that user should use Smart Location Bar for searching web. So, changing input mode in Smart Location Bar really doesn't make sense now.

Additionally, Smart Location Bar works as search bar for bookmarks and history. So, only having "url" as inputmode is too odd.

Unfortunately, there is no proper inputmode value in the spec:
https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#input-modalities:-the-inputmode-attribute

Therefore, I'd like to suggest that Smart Location Bar should have a special inputmode value such as "mozAwesomebar". Then, both Windows and Android IME handler should treat the value as "URL" or something better value.

What do you think, Gijs?


[Tracking Requested - why for this release]:
New default Photon UI changes the Smart Location Bar meaning with dropping search bar. So, we need to fix the inputmode value of it.
I'm trying to write patches and make IME handler of Windows set input scopes as both IS_URL and IS_SEARCH. However, some IMEs make their open state to "open" forcibly. This is really not useful for such IME users if they want to type URL directly. (Such IMEs make their open state to "open" when I set focus to the search bar.)

So, perhaps, we should use IS_DEFAULT for the Smart Location Bar instead of IS_URL and IS_SEARCH.
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #1)
> I'm trying to write patches and make IME handler of Windows set input scopes
> as both IS_URL and IS_SEARCH. However, some IMEs make their open state to
> "open" forcibly.

Ah, sorry, I just hit a bug of Win10, sometimes IME state are confused. After rebooting my environment, works fine with specifying both IS_URL and IS_SEARCH.
Additionally, I don't think that a lot of users type URL directly with VKB. So, setting VKB to "url" must not help so many users. Do we have telemetry about the usage of Smart Location Bar?
Whiteboard: [fxsearch][triage]
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #3)
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=ff7244d56efe692b6ce959384a259e6af9264831

FWIW, I tried the build from this trypush, and on my win10 tablet it no longer brings up either the url or search OSK, it just brings up the regular one. I assume this isn't what you intended?


Can we detect in the frontend code whether a problematic IME is turned on, and avoid using the url input type then?
Flags: needinfo?(masayuki)
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #4)
> Additionally, I don't think that a lot of users type URL directly with VKB.
> So, setting VKB to "url" must not help so many users. Do we have telemetry
> about the usage of Smart Location Bar?

I doubt we have telemetry that can tell us e.g. how often people intentionally use the ".com" or "/" button on the VKB, or even how often their input includes a slash or the ".com" string (or some other domain gTLD suffix). Marco, can you confirm if we know anything like that?
Flags: needinfo?(mak77)
Nothing like that, collecting it may have privacy implications too, even if trivial ones.
Flags: needinfo?(mak77)
(In reply to :Gijs from comment #5)
> (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #3)
> > https://treeherder.mozilla.org/#/
> > jobs?repo=try&revision=ff7244d56efe692b6ce959384a259e6af9264831
> 
> FWIW, I tried the build from this trypush, and on my win10 tablet it no
> longer brings up either the url or search OSK, it just brings up the regular
> one. I assume this isn't what you intended?

Half of that, yes. But according to the design of Windows API, it depends on what Windows thinks when 2 or more input scopes are specified.

And I guess that it's not so important to show OSK for URL in a lot of cases, of course, it must be convenient in a few cases.

> Can we detect in the frontend code whether a problematic IME is turned on,
> and avoid using the url input type then?

It's possible with blacklist in TSF mode. However, it's difficult to check non-Japanese IMEs because I'm not familiar with Chinese (both Traditional and Simpified) 3rd party's IME share. But I guess this is only a problem with MS-IME for Japanese and Google Japanese Input since we've not got any reports with non-Japanese IMEs.

On the other hand, I still think that how the OSK for URL is important?
Flags: needinfo?(masayuki)
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #8)
> (In reply to :Gijs from comment #5)
> > (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #3)
> > > https://treeherder.mozilla.org/#/
> > > jobs?repo=try&revision=ff7244d56efe692b6ce959384a259e6af9264831
> > 
> > FWIW, I tried the build from this trypush, and on my win10 tablet it no
> > longer brings up either the url or search OSK, it just brings up the regular
> > one. I assume this isn't what you intended?
> 
> Half of that, yes.

Can you elaborate? Not sure which half you expected / didn't expect. :-)

> But according to the design of Windows API, it depends on
> what Windows thinks when 2 or more input scopes are specified.
> 
> And I guess that it's not so important to show OSK for URL in a lot of
> cases, of course, it must be convenient in a few cases.

Well, in English and other Latin languages the URL OSK is strictly better here than the other one. We don't lose any functionality, it's just more useful to have the URL one vs. the normal one. There aren't really any serious downsides (which I understand is not the case when using IME inputs that break/change in this case).

> > Can we detect in the frontend code whether a problematic IME is turned on,
> > and avoid using the url input type then?
> 
> It's possible with blacklist in TSF mode.

To be clear, I don't suggest we disable the IME (which is what "blacklist" sounds like - maybe I misunderstand?), I was thinking we could use some JS in the frontend based on whether the IME is turned on, and simply swap the inputmode attribute for something other than "url".

> However, it's difficult to check
> non-Japanese IMEs because I'm not familiar with Chinese (both Traditional
> and Simpified) 3rd party's IME share. But I guess this is only a problem
> with MS-IME for Japanese and Google Japanese Input since we've not got any
> reports with non-Japanese IMEs.

Perhaps folks from Taipei or Mozilla China can help here?

> On the other hand, I still think that how the OSK for URL is important?

I'm sorry, I don't quite know what you're asking. If you're asking "is it important to show the URL-type OSK for the url bar?", I would say "yes". Chrome and Edge (both of which have never had a separate search bar) also do this, we looked odd when we didn't, people filed bugs about it. Speaking of which, what do Edge/Chrome do with the MS-IME / Google Japanese Input IMEs?
Flags: needinfo?(masayuki)
(In reply to :Gijs from comment #9)
> (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #8)
> > (In reply to :Gijs from comment #5)
> > > (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #3)
> > > > https://treeherder.mozilla.org/#/
> > > > jobs?repo=try&revision=ff7244d56efe692b6ce959384a259e6af9264831
> > > 
> > > FWIW, I tried the build from this trypush, and on my win10 tablet it no
> > > longer brings up either the url or search OSK, it just brings up the regular
> > > one. I assume this isn't what you intended?
> > 
> > Half of that, yes.
> 
> Can you elaborate? Not sure which half you expected / didn't expect. :-)

Well, I just intend to notify OS/IME/OSK of _all_ hints of current input context. So, it's not my intention which OSK is shown.

> > But according to the design of Windows API, it depends on
> > what Windows thinks when 2 or more input scopes are specified.
> > 
> > And I guess that it's not so important to show OSK for URL in a lot of
> > cases, of course, it must be convenient in a few cases.
> 
> Well, in English and other Latin languages the URL OSK is strictly better
> here than the other one. We don't lose any functionality, it's just more
> useful to have the URL one vs. the normal one. There aren't really any
> serious downsides (which I understand is not the case when using IME inputs
> that break/change in this case).

Okay. So, OSK for "search" doesn't have any better feature but one for "URL" does. Is this right? Then, it's reasonable to show OSK for "URL".

> > > Can we detect in the frontend code whether a problematic IME is turned on,
> > > and avoid using the url input type then?
> > 
> > It's possible with blacklist in TSF mode.
> 
> To be clear, I don't suggest we disable the IME (which is what "blacklist"
> sounds like - maybe I misunderstand?), I was thinking we could use some JS
> in the frontend based on whether the IME is turned on, and simply swap the
> inputmode attribute for something other than "url".

Yeah, of course. In TSF mode, we can check active IME strictly.  Therefore, we can use IS_DEFAULT only for them (I said this is decided with a blacklist). Otherwise, can use IS_URL.

> > However, it's difficult to check
> > non-Japanese IMEs because I'm not familiar with Chinese (both Traditional
> > and Simpified) 3rd party's IME share. But I guess this is only a problem
> > with MS-IME for Japanese and Google Japanese Input since we've not got any
> > reports with non-Japanese IMEs.
> 
> Perhaps folks from Taipei or Mozilla China can help here?

Yeah, but at least, we can keep current behavior with the above approach.

> > On the other hand, I still think that how the OSK for URL is important?
> 
> I'm sorry, I don't quite know what you're asking. If you're asking "is it
> important to show the URL-type OSK for the url bar?", I would say "yes".
> Chrome and Edge (both of which have never had a separate search bar) also do
> this, we looked odd when we didn't, people filed bugs about it.

Okay, I understand.

> Speaking of
> which, what do Edge/Chrome do with the MS-IME / Google Japanese Input IMEs?

I'm not sure about Edge because in my environment, I cannot use IME in URL bar with those IMEs. I guess Edge has some bugs for non-default IME.  Oddly, Chrome does nothing for Google Japanese Input nor MS-IME for Japanese. It might hack something with checking OSK visible state.
Flags: needinfo?(masayuki)
(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #10)
> (In reply to :Gijs from comment #9)
> > (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #8)
> > > But according to the design of Windows API, it depends on
> > > what Windows thinks when 2 or more input scopes are specified.
> > > 
> > > And I guess that it's not so important to show OSK for URL in a lot of
> > > cases, of course, it must be convenient in a few cases.
> > 
> > Well, in English and other Latin languages the URL OSK is strictly better
> > here than the other one. We don't lose any functionality, it's just more
> > useful to have the URL one vs. the normal one. There aren't really any
> > serious downsides (which I understand is not the case when using IME inputs
> > that break/change in this case).
> 
> Okay. So, OSK for "search" doesn't have any better feature but one for "URL"
> does. Is this right? Then, it's reasonable to show OSK for "URL".

Yeah, I think so. Search only shows a search icon on the OSK in the place where "enter/return" normally appears. I don't think there are other differences between "search" and the "default" keyboard shown in the OSK.

> > > > Can we detect in the frontend code whether a problematic IME is turned on,
> > > > and avoid using the url input type then?
> > > 
> > > It's possible with blacklist in TSF mode.
> > 
> > To be clear, I don't suggest we disable the IME (which is what "blacklist"
> > sounds like - maybe I misunderstand?), I was thinking we could use some JS
> > in the frontend based on whether the IME is turned on, and simply swap the
> > inputmode attribute for something other than "url".
> 
> Yeah, of course. In TSF mode, we can check active IME strictly.  Therefore,
> we can use IS_DEFAULT only for them (I said this is decided with a
> blacklist). Otherwise, can use IS_URL.

This sounds good to me, certainly as a  mitigation for 57. We can iterate if we find better alternatives that work - there's a lot of moving parts (frontend, core/tsf/ime code, several potential external IMEs, windows osk on 2 different versions of windows, ...) which makes it difficult to find the perfect solution.
# Due to bug 1405211, I cannot use MozReview for this bug.

Smart Location Bar (a.k.a URL bar) has some features, loading inputted URL
directly, searching bookmark items and history items, and search inputted words
with registered search engine.

So, it does not make sense its inputmode is "url".  E.g., neither showing URL
specific software keyboard nor switching IME open state automatically for
typing URL may not be expected in most cases.

Unfortunately, there is no proper inputmode value for Smart Location Bar.
Therefore, this patch uses "mozAwesomebar" value and accepts the value only in
chrome documents.  This value should be handled by each native IME handler
properly.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Attachment #8914647 - Flags: review?(gijskruitbosch+bugs)
Attachment #8914647 - Flags: review?(bugs)
When "mozAwesomebar" is set to inputmode value, that means that the Smart
Location Bar gets focus.  In that case, we should notify IME of input scopes
as "URL" because on-screen keyboard for URL has some useful additional keys
but they are not hindrances even when users want to type non-URL text.

On the other hand, MS-IME for Japanese and Google Japanese Input changes their
open state to "closed" if we notify them of URL input scope.  A lot of users
complain about this behavior.  Therefore, we should notify only them of
"Default" input scope even when "mozAwesomebar" has focus.
Attachment #8914648 - Flags: review?(m_kato)
Attachment #8914648 - Flags: review?(gijskruitbosch+bugs)
Although, I'm not sure if Firefox for Android uses urlbarBindings.xml for
declaring its awesome bar, for consistency with widget code for desktop OSes,
GeckoInputConnection should treat "mozAwesomebar" inputmode value as "url"
since Android doesn't have any special input type for "search" and we should
keep current behavior.
Attachment #8914650 - Flags: review?(nchen)
Attachment #8914650 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8914647 [details] [diff] [review]
Bug 1404206 - part1: Smart Location Bar should have special inputmode value, mozAwesomebar

What is "Smart Location Bar"?
Isn't it usually called Awesomebar.

(When I saw the bug title, I thought first this is about some addon ;) )
Attachment #8914647 - Flags: review?(bugs) → review+
Hmm, in my understanding, the official name for marketing is "Smart Location Bar" and the project name was "Awesomebar" and still called so by developers. However, "Awesome bar" is used in the help document:
https://support.mozilla.org/en-US/kb/awesome-bar-search-firefox-bookmarks-history-tabs
Comment on attachment 8914650 [details] [diff] [review]
Bug 1404206 - part3: Make GeckoInputConnection handle "mozAwesomebar" inputmode value as "url"

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

This is fine, though Android uses a native widget for the URL bar so GeckoInputConnection is not involved at all.
Attachment #8914650 - Flags: review?(nchen) → review+
Comment on attachment 8914647 [details] [diff] [review]
Bug 1404206 - part1: Smart Location Bar should have special inputmode value, mozAwesomebar

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

::: dom/events/IMEStateManager.cpp
@@ +1312,5 @@
>                          context.mHTMLInputInputmode);
> +      if (context.mHTMLInputInputmode.EqualsLiteral("mozAwesomebar") &&
> +          !nsContentUtils::IsChromeDoc(aContent->OwnerDoc())) {
> +        // mozAwesomebar should be allowed only in chrome
> +        context.mHTMLInputInputmode.Truncate();

I'm about the furthest you could get from an expert in this code, but perhaps the above should use GetEnumAttr or GetInputMode() directly to avoid random text in this mHTMLInputInputmode member, in the non-chrome-doc case? That's what the DOM code seems to do. Maybe followup fodder if I'm right.
Attachment #8914647 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8914648 [details] [diff] [review]
Bug 1404206 - part2: Make TSFTextStore and IMEHandler handle "mozAwesomebar" inputmode value

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

I still haven't tested this (working on a local build) but the code looks good to me. Thank you for the extensive comments.

::: widget/windows/TSFTextStore.cpp
@@ +3641,5 @@
> +      // FYI: Google Japanese Input may be an IMM-IME.  If it's installed on
> +      //      Win7, it's always IMM-IME.  Otherwise, basically, it's a TIP.
> +      //      However, if it's installed on Win7 and has not been updated yet
> +      //      after the OS is upgraded to Win8 or later, it's still an IMM-IME.
> +      //      Therefore, we also need to check with IMMHandler here.

Wow. Just, wow. I can't check any of this, but I'm impressed.

@@ +3644,5 @@
> +      //      after the OS is upgraded to Win8 or later, it's still an IMM-IME.
> +      //      Therefore, we also need to check with IMMHandler here.
> +      if (TSFStaticSink::IsMSJapaneseIMEActive() ||
> +          TSFStaticSink::IsGoogleJapaneseInputActive() ||
> +          IMMHandler::IsGoogleJapaneseInputActive()) {

I take it these also don't respond well for IS_SEARCH?

::: widget/windows/WinIMEHandler.cpp
@@ +636,5 @@
> +        arraySize = ArrayLength(inputScopes);
> +      } else {
> +        static const InputScope inputScopes[] = { IS_URL };
> +        scopes = &inputScopes[0];
> +        arraySize = ArrayLength(inputScopes);

Sometime, not in this bug, it would be so nice if we could make this code slightly less verbose. All these blocks basically do the same thing. Maybe we can use a local define or macro or something to reduce the duplication.
Attachment #8914648 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8914650 [details] [diff] [review]
Bug 1404206 - part3: Make GeckoInputConnection handle "mozAwesomebar" inputmode value as "url"

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

Don't think my review is helpful for this part (I mostly don't know android), but r=me given nchen's review, though see comment below.

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoInputConnection.java
@@ +556,5 @@
>              outAttrs.inputType |= InputType.TYPE_TEXT_VARIATION_PASSWORD;
>          else if (mIMEState == IME_STATE_PLUGIN)
>              outAttrs.inputType = InputType.TYPE_NULL; // "send key events" mode
> +        else if (mIMETypeHint.equalsIgnoreCase("url") ||
> +                 mIMETypeHint.equalsIgnoreCase("mozAwesomebar"))

Android doesn't use urlbarBindings.xml. Are we sure this doesn't accidentally apply to content stuff? Or is that fed through the IMEState code from patch #1 which would have filtered it out if this wasn't a chrome doc?
Attachment #8914650 - Flags: review?(gijskruitbosch+bugs) → review+
Tested on my Surface Pro 3, and it continues to show the url keyboard there. \o/
(In reply to Jim Chen [:jchen] [:darchons] from comment #19)
> Comment on attachment 8914650 [details] [diff] [review]
> Bug 1404206 - part3: Make GeckoInputConnection handle "mozAwesomebar"
> inputmode value as "url"
> 
> Review of attachment 8914650 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This is fine, though Android uses a native widget for the URL bar so
> GeckoInputConnection is not involved at all.

Thank you. I'll update the commit message before landing.

(In reply to :Gijs from comment #20)
> Comment on attachment 8914647 [details] [diff] [review]
> Bug 1404206 - part1: Smart Location Bar should have special inputmode value,
> mozAwesomebar
> 
> Review of attachment 8914647 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/events/IMEStateManager.cpp
> @@ +1312,5 @@
> >                          context.mHTMLInputInputmode);
> > +      if (context.mHTMLInputInputmode.EqualsLiteral("mozAwesomebar") &&
> > +          !nsContentUtils::IsChromeDoc(aContent->OwnerDoc())) {
> > +        // mozAwesomebar should be allowed only in chrome
> > +        context.mHTMLInputInputmode.Truncate();
> 
> I'm about the furthest you could get from an expert in this code, but
> perhaps the above should use GetEnumAttr or GetInputMode() directly to avoid
> random text in this mHTMLInputInputmode member, in the non-chrome-doc case?
> That's what the DOM code seems to do. Maybe followup fodder if I'm right.

Well, looks like that not all inputmode values haven't been treated as enum yet:
https://searchfox.org/mozilla-central/rev/e62604a97286f49993eb29c493672c17c7398da9/dom/html/HTMLInputElement.cpp#189-201
So, perhaps, I should file a follow up bug. Anyway, I guess that using nsString in InputContext may cause some damage of runtime performance.

(In reply to :Gijs from comment #21)
> Comment on attachment 8914648 [details] [diff] [review]
> Bug 1404206 - part2: Make TSFTextStore and IMEHandler handle "mozAwesomebar"
> inputmode value
> 
> Review of attachment 8914648 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I still haven't tested this (working on a local build) but the code looks
> good to me. Thank you for the extensive comments.

Yeah, I tested about Japanese IMEs with VMs, don't worry about that.
# Not yet on Win8.1, but I'll do that before landing.

> ::: widget/windows/TSFTextStore.cpp
> @@ +3641,5 @@
> > +      // FYI: Google Japanese Input may be an IMM-IME.  If it's installed on
> > +      //      Win7, it's always IMM-IME.  Otherwise, basically, it's a TIP.
> > +      //      However, if it's installed on Win7 and has not been updated yet
> > +      //      after the OS is upgraded to Win8 or later, it's still an IMM-IME.
> > +      //      Therefore, we also need to check with IMMHandler here.
> 
> Wow. Just, wow. I can't check any of this, but I'm impressed.

Well, TSF became almost stable at Win7, but perhaps, still had some trouble at that time. Therefore, they might faking it as IMM-IME in Win7...

> @@ +3644,5 @@
> > +      //      after the OS is upgraded to Win8 or later, it's still an IMM-IME.
> > +      //      Therefore, we also need to check with IMMHandler here.
> > +      if (TSFStaticSink::IsMSJapaneseIMEActive() ||
> > +          TSFStaticSink::IsGoogleJapaneseInputActive() ||
> > +          IMMHandler::IsGoogleJapaneseInputActive()) {
> 
> I take it these also don't respond well for IS_SEARCH?

Well, IS_SEARCH doesn't cause making MS-IME for Japanese nor Google Japanese Input does anything special. So, we can set IS_SEARCH as the input scope. However, we don't set IS_SEARCH when IS_URL is available. So, I don't want to specify IS_SEARCH only in this case for avoiding confusion of other developers.  If this reason doesn't make sense for you or using IS_SEARCH has some advantage for OSK, let me know. I don't mind to use IS_SEARCH in this case.

> ::: widget/windows/WinIMEHandler.cpp
> @@ +636,5 @@
> > +        arraySize = ArrayLength(inputScopes);
> > +      } else {
> > +        static const InputScope inputScopes[] = { IS_URL };
> > +        scopes = &inputScopes[0];
> > +        arraySize = ArrayLength(inputScopes);
> 
> Sometime, not in this bug, it would be so nice if we could make this code
> slightly less verbose. All these blocks basically do the same thing. Maybe
> we can use a local define or macro or something to reduce the duplication.

Yeah, but I'd like to keep the change as far as minimum for reducing the risk and taking it on Beta.

(In reply to :Gijs from comment #23)
> Tested on my Surface Pro 3, and it continues to show the url keyboard there.
> \o/

Thank you, that's what I cannot test on my environment!
(In reply to :Gijs from comment #22)
> Comment on attachment 8914650 [details] [diff] [review]
> Bug 1404206 - part3: Make GeckoInputConnection handle "mozAwesomebar"
> inputmode value as "url"
> 
> Review of attachment 8914650 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Don't think my review is helpful for this part (I mostly don't know
> android), but r=me given nchen's review, though see comment below.
> 
> :::
> mobile/android/geckoview/src/main/java/org/mozilla/gecko/
> GeckoInputConnection.java
> @@ +556,5 @@
> >              outAttrs.inputType |= InputType.TYPE_TEXT_VARIATION_PASSWORD;
> >          else if (mIMEState == IME_STATE_PLUGIN)
> >              outAttrs.inputType = InputType.TYPE_NULL; // "send key events" mode
> > +        else if (mIMETypeHint.equalsIgnoreCase("url") ||
> > +                 mIMETypeHint.equalsIgnoreCase("mozAwesomebar"))
> 
> Android doesn't use urlbarBindings.xml. Are we sure this doesn't
> accidentally apply to content stuff? Or is that fed through the IMEState
> code from patch #1 which would have filtered it out if this wasn't a chrome
> doc?

Yes, the chrome doc check should make this safer. Anyway, inputmode in content is still disabled in default settings though.
Attachment #8914648 - Flags: review?(m_kato) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5af9e7f8e36
Part 1: Smart Location Bar should have special inputmode value, mozAwesomebar. r=smaug, r=gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f0a70dd5077
Part 2: Make TSFTextStore and IMEHandler handle "mozAwesomebar" inputmode value. r=m_kato, r=gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/5e12ee5d4151
Part 3: Make GeckoInputConnection handle "mozAwesomebar" inputmode value as "url". r=jchen, r=gijs
Keywords: checkin-needed
Comment on attachment 8915051 [details] [diff] [review]
part1: Smart Location Bar should have special inputmode value, mozAwesomebar (r=smaug, gijs)

Approval Request Comment
[Feature/Bug causing the regression]:
Not a recent regression about the issues of Japanese IMEs. However, this becomes serious issue for new profile since Photon doesn't include search bar in the navigation toolbar.

[User impact if declined]:
When Japanese users want to search something in the web from the URL bar, IME may make its open state automatically "closed". This is really annoying if the user wants to search with Japanese words.

[Is this code covered by automated tests?]:
No.

[Has the fix been verified in Nightly?]:
Yes.

[Needs manual test from QE? If yes, steps to reproduce]:
1-1. Install MS-IME for Japanese on Windows from language settings.
1-2. Choose MS-IME for Japanese as active keyboard layout with Win+Space.
1-3. Focus <input> in web contents or search bar, then, open its open state with Alt + `. Then, type "a" and confirm "あ" is inputted. Then, commit the character with typing Enter key.
1-4. Click URL bar, and type "a".

Then, should be typed as "あ" rather than "a".

2-1. Choose non-Japanese language as active keyboard layout with Win+Space.
2-2. Open on-screen keyboard (touch keyboard) of Windows (not "screen keyboard").
2-3. Focus URL bar.

Then, on-screen keyboard should be in URL mode. E.g., having ".com" key. It depends on active keyboard layout's language.

[List of other uplifts needed for the feature/fix]:
The following patches.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
This patch changes inputmode attribute of URL bar. That's been never accessible from add-ons because XUL add-ons are never available.

[String changes made/needed]:
No.
Attachment #8915051 - Flags: approval-mozilla-beta?
Comment on attachment 8915053 [details] [diff] [review]
part2: Make TSFTextStore and IMEHandler handle "mozAwesomebar" inputmode value (r=m_kato, gijs)

Approval Request Comment
[Is the change risky?]:
Not so risky.

[Why is the change risky/not risky?]:
This patch only changes the behavior when MS-IME for Japanese or Google Japanese Input is active. I tested both on Win10, Win8.1 and Win7 before landing.

[String changes made/needed]:
No.
Attachment #8915053 - Flags: approval-mozilla-beta?
Comment on attachment 8915054 [details] [diff] [review]
part3: Make GeckoInputConnection handle "mozAwesomebar" inputmode value as "url" (r=jchen, gijs)

Approval Request Comment
[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
This doesn't change any behavior on Android and just keeps consistency of handling inputmode between Android and Windows's widget code. (Note that on the other platforms, native IME handler doesn't notify IME of inputmode information.)

So, even without this patch, should work the previous patches though. But I recommend to uplift this for keeping same code between 57 and 58.

[String changes made/needed]:
No.
Attachment #8915054 - Flags: approval-mozilla-beta?
Hi David, Justin, can I get a second opinion on whether this is a must fix for 57? I am seeing way more uplifts in Beta57 than usual. This will add a huge combined risk to ship on time and with expected quality.
Flags: needinfo?(dolske)
Flags: needinfo?(dbolter)
(In reply to Ritu Kothari (:ritu) from comment #34)
> Hi David, Justin, can I get a second opinion on whether this is a must fix
> for 57? I am seeing way more uplifts in Beta57 than usual. This will add a
> huge combined risk to ship on time and with expected quality.

FWIW, I don't think the pre-patch state of 57 without a separate searchbox is shippable for the Japanese market. Searching from the URL bar is almost impossible with the main input methods that Japanese users would use. We wouldn't win over any users from Chrome/Edge/IE (and maybe lose some), purely based on the location bar just being very hard to use.

Changing back to shipping a separate search box only in that market seems undesirable and would equally introduce additional risk to release quality, though you could check with Paolo/Panos to see if just flipping the pref for it for the Japanese localized version would be sufficient -- though that would not help users in those markets that use the en-US version of Firefox.

I will be the first to admit I'm not an expert in the IME world of things, but I *have* worked on several of our bugs to make the "touchscreen / tablet mode on windows 10" experience reasonable in the past. Based on those patches (which in some cases also got stuck into beta in order to make release dates), and having seen the work Masayuki has done here (with all the patches having been reviewed by 2 separate people) and having both reviewed + tested the patches myself, I am pretty confident that taking this patchset for 57 is going to be better than not taking it.

I also believe QA is already putting focus on doing testing relating to touch screens for this release, so I think regressions from this patchset should surface in that testing.

If it helps, I do also think the number of photon-related uplifts is slowing, at least in the bits I'm habitually involved in. :-)
Looking at the patches, Masayuki's risk assessment, and Gijs' very helpful comment 35 I think we should strongly consider taking this. Are there any known possible interactions or side-effects we should add to the testing plan?
Flags: needinfo?(dbolter)
One of known side-effects is, on-screen keyboard for MS-IME for Japanese and Google Japanese Input user is shown in normal mode instead of URL mode. However, it's intentional behavior for now. Showing some special keys in on-screen keyboard isn't as important as avoiding unexpected open state change.
Tracking based on comments 35/36.
Comment on attachment 8915051 [details] [diff] [review]
part1: Smart Location Bar should have special inputmode value, mozAwesomebar (r=smaug, gijs)

Thanks Gijs, David for your assessment. Let's take it, Beta57+
Attachment #8915051 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8915053 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8915054 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: needinfo?(dolske)
You need to log in before you can comment on or make changes to this bug.