Doug Sherk (:drs) (inactive) Assignee
	Description • 10 years ago

We broke something when fixing a test for the focus() event on the SIM picker. I believe it's here: https://github.com/mozilla-b2g/gaia/blob/master/shared/js/sim_picker.js#L55

The bug in the FIXME is for a tech debt issue as opposed to actual bustage, so it's different.

	Julien Wajsberg [:julienw]
	Comment 1 • 10 years ago

Yeah, I think you really need to call "focus" ;)

In the test, you can simply spy on this function to check it's called?

	Doug Sherk (:drs) (inactive) Assignee
	Comment 2 • 10 years ago

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #1)
> Yeah, I think you really need to call "focus" ;)
> 
> In the test, you can simply spy on this function to check it's called?

That was what I did originally, but Anthony was opposed to it. His reasoning was that we're not testing whether the element is actually being focused on or not, only that the .focus() method is being called. We had trouble hooking the event that was sent through the DOM when calling .focus(), so we switched to this method.

	Julien Wajsberg [:julienw]
	Comment 3 • 10 years ago

Sometimes we need compromises though.

	Doug Sherk (:drs) (inactive) Assignee
	Comment 4 • 10 years ago

(In reply to Julien Wajsberg [:julienw] (away until March 24) from comment #3)
> Sometimes we need compromises though.

Yep! It turns out that even if the target element has no tabIndex or otherwise can't be focused on, it still receives the CustomEvent('focus'), so we're basically no better off by using that method instead of the focus() method.

	Jason Smith [:jsmith]
	Comment 5 • 10 years ago

Doug - Two questions:

1. Can you provide a screenshot of the bug?
2. Is bug 987044 a dupe of this bug?

	Doug Sherk (:drs) (inactive) Assignee
	Comment 6 • 10 years ago

(In reply to Jason Smith [:jsmith] from comment #5)
> Doug - Two questions:
> 
> 1. Can you provide a screenshot of the bug?
> 2. Is bug 987044 a dupe of this bug?

1. Here you go.
2. No, that's a different issue.

	Jason Smith [:jsmith]
	Comment 7 • 10 years ago

So the impact of the bug is the fact that the keyboard persists if the SIM picker dialog is open. That's certainly a bug, but I don't think I'd block on it mainly because the only impact is the fact that there's irrelevant UI present here. We should get a fix here though for 1.4, so I'd highly suggest asking for 1.4 approval when this patch gets a r+.

	Julien Wajsberg [:julienw]
	Comment 8 • 10 years ago

Comment on attachment 8395757 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/17534

looks goo

r=me granted that you tested the patch on a real device (I didn't).

Please request approval to land this on v1.4.

	Doug Sherk (:drs) (inactive) Assignee
	Comment 9 • 10 years ago

https://github.com/mozilla-b2g/gaia/commit/47f22769144b52a2c27090900a120db029c771de

	Doug Sherk (:drs) (inactive) Assignee
	Comment 11 • 10 years ago

Comment on attachment 8395757 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/17534

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 947140
[User impact] if declined: Keyboard will not be hidden when opening DSDS SIM picker in the SMS app.
[Testing completed]: Ran on Fugu device and verified the fix myself.
[Risk to taking this patch] (and alternatives if risky): Low.
[String changes made]: None.

	bhavana bajaj [:bajaj]
	Comment 12 • 10 years ago

Comment on attachment 8395757 [details]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/17534

Low risk change which avoids irrelevant UI. Thanks for the tests!

	Doug Sherk (:drs) (inactive) Assignee
	Comment 13 • 10 years ago

v1.4: https://github.com/mozilla-b2g/gaia/commit/bfeca40c4dd4d42b051434d828df6db2432fd072