877352 - Defect - There's no way to dismiss the soft keyboard from Firefox Start

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: Firefox Start, defect, P1)

Description

[Jim Mathies [:jimm]]

str:

1) open tab / start page
2) tap on nav bar edit (keyboard comes up)

try to get rid of the keyboard by tapping on empty space, you can't.

Comment 1

[Jim Mathies [:jimm]]

Attachment: patch

Comment 2

[Marco Mucci [:MarcoM]]

Hey Jim, what would you like to set the point estimate at?

Comment 3

[Rodrigo Silveira [:rsilveira] [:rodms]]

Comment on attachment 755569 [details] [diff] [review]
patch

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

I'm concerned with relying on the class name to know if you should dismiss.

::: browser/metro/base/content/browser-ui.js
@@ +1489,5 @@
>        this.hide(aURI);
>      }
>    },
>  
> +  hasClass: function hasClass(aClassString, aClassList) {

You can use classList instead: https://developer.mozilla.org/en-US/docs/Web/API/element.classList

@@ +1504,5 @@
> +    // focus from the nav bar edit so the soft keyboard will hide.
> +    let className = aEvent.originalTarget.className;
> +    let classList = ["richgrid-grid",
> +                     "meta-section-title",
> +                     "scrollbox-innerbox"];

Checking for the classes seem very fragile. We could stop propagation when you click on a tile and always blur the textbox when we get here.

We could also use pointer-events: none on everything except the tiles and #start-container, then just check if the target is #start-container or stop propagation when clicking on the tile. This is more cumbersome though.

@@ +1506,5 @@
> +    let classList = ["richgrid-grid",
> +                     "meta-section-title",
> +                     "scrollbox-innerbox"];
> +    if (this.hasClass(aEvent.originalTarget.className, classList)) {
> +      BrowserUI._edit.blur();

Not really part of this bug, but we also need to dismiss the context bar. If you don't want to handle this here I'll open another bug if it doesn't exist yet.

Comment 4

[Jim Mathies [:jimm]]

(In reply to Rodrigo Silveira [:rsilveira] from comment #3)
> Checking for the classes seem very fragile. We could stop propagation when
> you click on a tile and always blur the textbox when we get here.

Yes I considered this, will take a shot at it. Making sure every click target in the grid stops propagation seemed fragile too though. But maybe there's a way to catch this generically. All we currently have are grid tiles and the titles afaik.

> Not really part of this bug, but we also need to dismiss the context bar. If
> you don't want to handle this here I'll open another bug if it doesn't exist
> yet.

I think we want this to stick around if items are selected in the grid, which is why it's visible?

Comment 5

[Jim Mathies [:jimm]]

Attachment: fix v.2

This wasn't too bad. I left the title labels alone, so those bubble up and dismiss. The tiles are the only elements that trap at this point. Seems to work pretty well.

Comment 6

[Jim Mathies [:jimm]]

Attachment: fix v.3

Slight improvement, add a method to BrowserUI for doing this.

Comment 7

[Rodrigo Silveira [:rsilveira] [:rodms]]

Comment on attachment 755910 [details] [diff] [review]
fix v.3

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

Looks great.

Comment 8

[Jim Mathies [:jimm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/54d960e4e883

Thanks!

Comment 9

[Rodrigo Silveira [:rsilveira] [:rodms]]

(In reply to Jim Mathies [:jimm] from comment #4)
> > Not really part of this bug, but we also need to dismiss the context bar. If
> > you don't want to handle this here I'll open another bug if it doesn't exist
> > yet.
> 
> I think we want this to stick around if items are selected in the grid,
> which is why it's visible?

Yes, I was looking at how windows 8 start behaves and an edge swipe from the top unselects all tiles and hides the context bar. The case that I think is more annoying is when you have an undo action button on the context bar because the only way to dismiss is to select something then unselect it.

I'll open a bug for this.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/54d960e4e883

Comment 11

[juan becerra [:juanb]]

Tested on 2013-06-14 using the latest nightly built from http://hg.mozilla.org/mozilla-central/rev/b197bed90a98

- Tested using the steps in comment #0 and now I can consistently dismiss the soft keyboard from the Firefox Start page.
- Tested opening a new tab, focusing on the location bar, then tapping on an empty area in the Firefox Start page, and that dismissed the soft keyboard
- Tested focusing on the location bar, typing something with the soft keyboard, tapping on an empty area, dismissing the keyboard, tapping again on the location, bar and so on.

Comment 12

[Kamil Jozwiak [:kjozwiak]]

Went through the following "Defect" for iteration #9 testing without any issues. Used the following build:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-06-25-03-12-38-mozilla-central/

- Went through the original issue from comment 0 without any issues
- Went through the test cases that have been added in comment 11 without any issues
- Ensured that sliding another application from the left hand side auto dismisses the OSK while on the awesome screen
- Tapping and focusing on the "Location Bar" and then tapping on a tile from "Top Sites" and ensuring that the OSK is dismissed as a new tab is created
- Tapping and focusing on the "Location Bar" and then tapping on a tile from "Bookmarks" and ensuring that the OSK is dismissed as a new tab is created
- Tapping and focusing on the "Location Bar" and then tapping on a tile from "Recent History" and ensuring that the OSK is dismissed as a new tab is created

Comment 13

[Kamil Jozwiak [:kjozwiak]]

Went through the following "Defect" for iteration #10 testing without any issues. Used the following build:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-07-19-03-02-04-mozilla-central/

- Went through the original test case from comment 0 without any issues
- Went through the test cases added in comment 11 without any issues
- Went through the test cases added in comment 12 without any issues

Comment 14

[Samvedana (:Samvedana)]

User Agent: Mozilla/5.0 (Windows NT 6.2; Win64; x64; rv:26.0) Gecko/20100101 Firefox/26.0
Build ID: 20130807030216
Built from http://hg.mozilla.org/mozilla-central/rev/1fb5d14e8348

WFM
Tested on windows 8 using latest nightly  for iteration-11. Followed steps provided in comment0 and got expected result.

Comment 15

[Samvedana (:Samvedana)]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0
Build ID: 20130816030205
Built from http://hg.mozilla.org/mozilla-central/rev/1ed5a88cd4d0

Didn't WFM
Tested on windows 8 using latest nightly for iteration-12. Followed steps provided in comment0 and I am still seeing same issue.

Comment 16

[Kamil Jozwiak [:kjozwiak]]

Went through the following "Defect" for iteration #15 testing without any issues. Used the following build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2013-10-02-03-02-01-mozilla-central/

- Went through the original test case from comment 0 without any issues
- Went through the test cases added in comment 11 without any issues
- Went through the test cases added in comment 12 without any issues
- Went through all of the above test cases in full & filled views without any issues