1168600 - javascript prompt() not working properly

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::System UI, defect, P2)

Description

[Jamie Charlton (:Jamie_)]

Attachment: video of the test case

when using prompt, when you hit enter instead of click the okay button the variable that the prompt is being set to gets null instead of the prompted value.

https://github.com/1Jamie/test-app

if you run that app on a device or simulator when you click the button in the app it prompts you for text. If you select the okay it pushes the entered value into a notification, on the other hand if you hit the enter key it puts in a value of null in the notification

If you are running it on a computer browser it all works correctly. If you hit the button a prompt opens and you can either hit okay or hit the enter key and it pushes the entered value into a notification.

see attached video to see what is being said in this bug.

tested on both simulator and device with same results

Environmental Variables:
Device: Flame 3.0
Build ID: 20150524195401
Gaia: 5bcc08a732163087999251b523e3643db397412c
Gecko: b6623a27fa64
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 41.0a1 (3.0)
Firmware Version: v18D_nightly_v2
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

device: simulator
apptype	b2g
vendor	Mozilla
name	B2G
version	3.0.0.0-prerelease
appbuildid	20150526101844
platformbuildid	20150526101844
platformversion	41.0a1
geckobuildid	20150526101844
geckoversion	41.0a1
changeset	e537a1ba501b
locale	en-US
os	B2G

Comment 1

[Jamie Charlton (:Jamie_)]

Can you take a look at this please

Comment 2

[Brogan Zumwalt [Inactive]]

I can get this to reproduce across Flame 2.0, 2.1, 2.2, and 3.0. Also reproduces on the latest Aries/Spark 3.0. Prompt() method functions properly when pressing confirmation button in app, but when user presses enter on keyboard null text is returned.

This can also be reproduced outside of app in browser (http://www.w3schools.com/jsref/tryit.asp?filename=tryjsref_prompt)

Device: Flame 2.0
Build ID: 20150526000203
Gaia: 5552bf529d3d6775a968942e9afa6c1d4037362c
Gecko: b841c3b8a094
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 32.0 (2.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Device: Flame 2.1
Build ID: 20150526001202
Gaia: 0d343f49c0e0a3928a4f456faf98e37bb26a6ef6
Gecko: 6e357036c54b
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 34.0 (2.1)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2
Build ID: 20150526002558
Gaia: 6a8d171d00efe8b27cba91bf1d789ab824579664
Gecko: 46f6c7327ab0
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Device: Flame 3.0
Build ID: 20150526010203
Gaia: 7cd4130d4f988562a77d126860408ada65bb95ef
Gecko: 43f2f0c506ea
Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67
Version: 41.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Device: Xperia Z3 Compact (B2G) 3.0
Build ID: 20150526104521
Gaia: 0359bf81f96a5356dacfc07e12ca72df61cbd638
Gecko: e537a1ba501b
Gonk: Could not pull gonk.  Did you shallow Flash?
Version: 41.0a1 (3.0)
User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0

Comment 3

[Jamie Charlton (:Jamie_)]

The app was just used as an easy example to show the concept of the problem.

Comment 4

[Gregor Wagner [:gwagner]]

This sounds pretty bad. Can we try to fix this for 2.2?

Comment 5

[[:fabrice] Fabrice Desré]

(In reply to Gregor Wagner [:gwagner] from comment #4)
> This sounds pretty bad. Can we try to fix this for 2.2?

Well, that can't happen on a touch only device...

Comment 6

[Gregor Wagner [:gwagner]]

(In reply to Fabrice Desré [:fabrice] from comment #5)
> (In reply to Gregor Wagner [:gwagner] from comment #4)
> > This sounds pretty bad. Can we try to fix this for 2.2?
> 
> Well, that can't happen on a touch only device...

I thought the return key on the soft keyboard triggers this as well.

Comment 7

[[:fabrice] Fabrice Desré]

(In reply to Gregor Wagner [:gwagner] from comment #6)
> (In reply to Fabrice Desré [:fabrice] from comment #5)
> > (In reply to Gregor Wagner [:gwagner] from comment #4)
> > > This sounds pretty bad. Can we try to fix this for 2.2?
> > 
> > Well, that can't happen on a touch only device...
> 
> I thought the return key on the soft keyboard triggers this as well.

Ha, I think you're right. Ignore me!

Comment 8

[Michael Henretty [:mikehenrty][:mhenretty]]

I'll take a look.

Comment 9

[Michael Henretty [:mikehenrty][:mhenretty]]

Tim, for some reason, the return key on the keyboard is triggering a click on the cancel button in the prompt dialog. Can you help me understand how the keyboard return event gets forwarded into the system app?

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Michael Henretty [:mhenretty] from comment #9)
> Tim, for some reason, the return key on the keyboard is triggering a click
> on the cancel button in the prompt dialog. Can you help me understand how
> the keyboard return event gets forwarded into the system app?

The keyboard key should be handled by the system prompt dialog as usual key events. There shouldn't be anything special.

Comment 11

[Jamie Charlton (:Jamie_)]

What is the current status of the situation of this bug?

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Thank you for bring this up. This bug was not properly prioritized since the last comment.

Let me turn a few flags on so it get triaged.

[Blocking Requested - why for this release]: this is not a regression for the current version, but we should try to fix it for web compatibility.

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Here is the root cause:

This [1] is the actual DOM [2] inserts.

[1]

         <form class="modal-dialog-prompt generic-dialog visible" role="dialog" tabindex="-1">
            <div class="modal-dialog-message-container inner">
              <h3 class="modal-dialog-prompt-title">UI tests</h3>
              <p>
                <span class="modal-dialog-prompt-message">Hello world:</span>
                <input class="modal-dialog-prompt-input">
              </p>
            </div>
            <menu data-items="2">
              <button class="modal-dialog-prompt-cancel cancel" data-l10n-id="cancel">Cancel</button>
              <button class="modal-dialog-prompt-ok confirm affirmative" data-l10n-id="ok">OK</button>
            </menu>
          </form>

[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_modal_dialog.js

The script binds click event handling functions on the two buttons.

When user hits [return] on the keyboard, it cause the form to submit. Since the first <button> is the cancel button, it in turn triggers the click handler of that button.

**It is important to remember the default type of a <button> is type=submit.** [3] Whoever design the DOM here forget that.

[3] https://html.spec.whatwg.org/multipage/forms.html#attr-button-type

So the quick fix here should be to (A) set the type of the cancel button as type=button, and so that the click of the confirm button will get triggered.

The alternative and better fix would be to instead (B) bind the event handler on <form onsubmit>, but the button types would still needs to be changed.

Anyone interested in submit a patch on (A) to confirm my theory?

Comment 14

[Mahendra Potharaju [:mahe]]

Blocks 2.5 with P2 priority. Has been around for quite sometime.

Comment 15

[GitHub Autolander Bot]

Attachment: [gaia] timdream:modal-dialog-prompt > mozilla-b2g:master

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8661616 [details] [review]
[gaia] timdream:modal-dialog-prompt > mozilla-b2g:master

I wonder if this should be protected with a Gij test?

Comment 17

[Alberto Pastor [:albertopq]]

Yes, I think it would make sense to add a Gij test. Thanks!

Comment 18

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8661616 [details] [review]
[gaia] timdream:modal-dialog-prompt > mozilla-b2g:master

Patch updated with Gij test.

Comment 19

[Alberto Pastor [:albertopq]]

Comment on attachment 8661616 [details] [review]
[gaia] timdream:modal-dialog-prompt > mozilla-b2g:master

LGTM! Thanks!

Comment 20

[Tim Guan-tin Chien [:timdream] (please needinfo)]

It is appears that the test cannot pass on our CI. I have rearrange it in a new commit but it still fails. Do you think we should land the patch w/o test disabled, or I should continue try to fix it?

Comment 21

[Alberto Pastor [:albertopq]]

I think there is a file missing (https://github.com/mozilla-b2g/gaia/pull/31860/files#diff-695c91813bebab2da685538d0e60e029R6) and that's the reason it doesn't work. I guess that script is adding the click listener to the dialog button, as right now nothing happens when clicking it

Comment 22

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Thanks for the reminder. I am embarrassed O_o.

master: https://github.com/mozilla-b2g/gaia/commit/637e2851ef119c4e29e176802d27e28b17c764c0