Closed Bug 793108 Opened 9 years ago Closed 9 years ago

value selection popup will show up again while re-entering app

Categories

(Firefox OS Graveyard :: General, defect, P3)

x86_64
Linux
defect

Tracking

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)

VERIFIED FIXED
blocking-basecamp +
Tracking Status
firefox18 --- fixed
firefox19 --- fixed

People

(Reporter: schien, Assigned: schien)

References

()

Details

Attachments

(2 files, 2 obsolete files)

re-post gaia issue #4686 to bugzilla since this bug should be fixed by platform.

STR:
1. Launch the clock app
2. Create a alarm
3. In the alarm edit page, click on the "Repeat" column
4. value selection popup for "Repeat" column shows up, then press OK button to go back to alarm edit page
5. Press home button to quit
6. Launch the clock app again

expected:
It will show the alarm edit page

actual:
value selection popup for "Repeat" column shows up again.
blocking-basecamp: --- → ?
In github issue #4686, Jonas suggest that blur event should be triggered after selection pop-up disappeared. Therefore, additional interface is required to sync the timing of removing focus from select element. Below are two possible approaches:

  1. Gaia invokes |MozKeyboard.cancelSelection()| while user clicks 'cancel' button on selection pop-up. Therefore, MozKeyboard can notify forms.js to remove focus during the invocation of MozKeyboard.setSelectedIndex() or MozKeyboard.cancelSelection().

  2. Gaia invokes |MozKeyboard.removeFocus()| while user clicks 'ok' or 'cancel button. MozKeyboard can notify forms.js to remove focus during the invocation of MozKeyboard.removeFocus().

However, by considering the use case of programmatically set focus on select element, pop-up a full-screen selection page will make user confuse since they cannot see which field is this selection page for. Therefore, the 3rd approach comes into my mind:

  3. for select element, forms.js should monitor click event instead of focus event.

  Which approach you guys prefer? Any other suggestion?
I don't think monitoring click events is the right solution since it means that applications that use javascript to call selectElement.focus() won't cause the popup to be shown. As a result I think a bunch of existing web content would stop working.

Instead I think we should call selectElement.blur() once the popup is closed by the user.
I don't think this is a blocker though. For the alarm app we can work around this by having the alarm app call .blur() from the change event.
blocking-basecamp: ? → -
@Jonas, "alarm app call .blur() from the change event" cannot fully resolve this problem because this issue still happens when value is not changed or even when user click cancel button.
(In reply to Jonas Sicking (:sicking) from comment #3)
> I don't think this is a blocker though. For the alarm app we can work around
> this by having the alarm app call .blur() from the change event.

Important note - this doesn't just affect the alarm app, it affects the date/time picker for the calendar app - see https://github.com/mozilla-b2g/gaia/issues/5483. Renoming per the comment on that bug. If I'm misinterpreting the comment in that bug, please explain why.
blocking-basecamp: - → ?
Let's try to get this fixed.

Mounir, is this something you can fix?
blocking-basecamp: ? → +
Component: General → Gaia
(In reply to Shih-Chiang Chien [:schien] from comment #4)
> @Jonas, "alarm app call .blur() from the change event" cannot fully resolve
> this problem because this issue still happens when value is not changed or
> even when user click cancel button.

We can surely fire |blur()| from mozKeyboard API to solve this issue in general, but the problem is that this is not how real web behaves.

Rudy and Ian have go through this with me the other day. Rudy or Ian, can one of you take the discussion and implement the solution?
Assignee: nobody → iliu
Priority: -- → P3
I propose that platform fire |blur()| from mozKeyboard API to solve this issue.

In mobile case, developer don't know and receive the event when user click "OK", "Cancel" without |change()| event(If user don't change any option really). 
So, if we fire |blur()| event, it will fix the issue and let developer to catch the check point with user's interaction.

If we don't have the |blur()| event for the check point, I propose to refine all value selector without using <select> tag.
We just customize the selector and listen the "OK", "Cancel" buttons.
It's also a big effort in gaia.
But the issue will be still there in the feature.
We could try to open a web page with <select> tag.
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #7)
> (In reply to Shih-Chiang Chien [:schien] from comment #4)
> > @Jonas, "alarm app call .blur() from the change event" cannot fully resolve
> > this problem because this issue still happens when value is not changed or
> > even when user click cancel button.
> 
> We can surely fire |blur()| from mozKeyboard API to solve this issue in
> general, but the problem is that this is not how real web behaves.

It's a good point that it's not exactly how the web normally works on desktop, but I dont think we can mimic exactly how desktop behaves given how different mobile input is.
I think we should go for this solution and see how well it works.
Duplicate of this bug: 796256
provide MozKeyboard.removeFocus() that allows gaia explicitly trigger blur in value_selector.js.
Basically, inovke |window.navigator.mozKeyboard.removeFocus()| in ValueSelector.confirm() and ValueSelector.cancel() will resolve this bug.
Attachment #670814 - Flags: feedback?(iliu)
NOTE: If blocking-basecamp+ is set, just land it for now.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky):

https://github.com/mozilla-b2g/gaia/pull/5810
Attachment #671332 - Flags: review?(rlu)
Attachment #671332 - Flags: feedback?(schien)
Comment on attachment 671332 [details]
Add removeFocus() for value selector

Bug 793108 - value selection popup will show up again while re-entering app

Add removeFocus() in ValueSelector.confirm() and ValueSelector.cancel().
I have test for the patch.
And it's work fine as Shih-Chiang Chien 's comment.

The patch is needed to merged with https://bugzilla.mozilla.org/attachment.cgi?id=670814 ,
after Jonas' confirmation.
Attachment #671332 - Flags: review?(rlu) → review+
Assignee: iliu → schien
Comment on attachment 670814 [details] [diff] [review]
WIP - add removeFocus() in MozKeyboard

According to comment 13, this patch is waiting for Jonas' confirmation on Keyboard API change.
Attachment #670814 - Flags: feedback?(jonas)
Comment on attachment 670814 [details] [diff] [review]
WIP - add removeFocus() in MozKeyboard

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

Yup, this sounds great.
Attachment #670814 - Flags: feedback?(jonas) → feedback+
patch clean-up for review.
Attachment #670814 - Attachment is obsolete: true
Attachment #670814 - Flags: feedback?(iliu)
Attachment #672728 - Flags: review?(jonas)
Duplicate of this bug: 802248
Comment on attachment 672728 [details] [diff] [review]
add removeFocus() in MozKeyboard, v1

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

r=me with the above fixed.

::: b2g/chrome/content/forms.js
@@ +48,5 @@
>  
>      if (this.focusedElement) {
>        this.focusedElement.removeEventListener('mousedown', this);
>        this.focusedElement.removeEventListener('mouseup', this);
> +      this.focusedElement.blur();

You should only call this if 'element' is null. Otherwise the events will be slightly different when focus is moving from one element to another.

When focus moves directly from one element to another you can in the blur event get information about where focus is moving. Blurring first and then focusing the new event would prevent that.
Attachment #672728 - Flags: review?(jonas) → review+
update patch and keep r+ according to comment 18.

Thanks Jonas! I didn't realize that we can detect newly focused element in blur event.
Attachment #672728 - Attachment is obsolete: true
Attachment #673118 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/08fa42148eab
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Why have we not simply clear the focus when the visibility of the window has changed?
If user press the home button while editing <input> or <select> element, we might as well display IME after re-entering app. However, IME should not appear if editing is completed. I don't think we have a clever way for distinguish these two cases in Gecko when the visibility of the window has changed. In addition, the unfocus approaches can also solve the cannot-edit-select-twice bug. Isn't it be great to solve two bugs with one simple patch. :)
Attachment #671332 - Flags: feedback?(schien)
verified on unagi
2013-01-08
gaia master: df13e23c775d02f430e01799ea8b19d61afb4b86
b2g-18: 9220d6eb3f77
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.