Closed Bug 908061 Opened 11 years ago Closed 11 years ago

[Youtube] Search does not start on pressing ENTER KEY

Categories

(Firefox OS Graveyard :: Gaia::Keyboard, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::Keyboard
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:koi+)

Status:
RESOLVED DUPLICATE of bug 918863
Milestone:
1.2 C4(Nov8)
Project Flags:
blocking-b2g koi+

People

(Reporter: leo.bugzilla.gaia, Unassigned)

Details

(Whiteboard: [3rd Party] [apps watch list1])

Attachments

(1 file, 1 obsolete file)

Images showing Issue
(deleted), application/zip
Details 
STR
========
1. Open Youtube app
2. Press Search icon (keyboard appears)
3. Type Something >> Press ENTER key in Keyboard
4.  

EXPECTED : Web page should load and Keyboard should hide

ACTUAL  : Web Page is loading but keyboard is not hiding
Flags: needinfo?(jsmith)
Flags: needinfo?(dpreston)
I need build information here. This has been fixed already before, so if this is happening now, it's a regression. Not a YouTube-specific issue either.
Blocks: 877024
No longer blocks: b2g-youtube
Component: Preinstalled B2G Apps → Gaia::Keyboard
Flags: needinfo?(jsmith)
Product: Tech Evangelism → Boot2Gecko
Attached file Images showing Issue (deleted) — 
Attached Images for the Issue.
Please check.
V1-train
Build Information:
20130821124253
Gaia Version : a83c5fee49525ec0545059b5a56a3fd9f1c09f19

I think this issue is not related to keyboard only. 
When I check in browser
Search >> type >> Press ENTER key >> keyboard hides and WebPage loads.
But when I do in case of Youtube(even from browser), it doesnt hide.
The keyboard is not getting the Hide Event.
I will put Rudy Lu in needinfo for his suggestion.
Please check from your side.

Thanks.
Flags: needinfo?(rlu)
If this triggered a page reload, then it should be a regression that keyboard would not be hidden.

The related code should live in Gecko's b2g/chrome/content/forms.js.
Flags: needinfo?(rlu)
I was able to repro this on the mozilla leo nightly build 20130903041201.

An interesting thing to note is that if the user presses the magnifying glass icon to search, the keyboard goes away properly, but if the user presses the return key on the keyboard, it does not.
Flags: needinfo?(dpreston)
Rudy, I think it is triggered on page reload, but I am not 100% sure. It could be ajax as well.
blocking-b2g: --- → leo?
Keywords: regression, regressionwindow-wanted
QA Contact: sparsons
This issue has been reproduced as far back as the 7/08 build on the Leo device. It happens on both Com and Moz ril.

Environmental Variables
Build ID: 20130708070214
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/8c0127cc7be3
Gaia: 7c40bdaeaffae708342fc773926dcfac5389348e
Platform Version: 18.1
RIL Version: 01.01.00.019.152 

The keyboard doesn't hide after performing a search on Youtube even after the search results appear.
Keywords: regressionwindow-wanted
QA Contact: sparsons
Maybe YouTube changed something on their side here. This was working before, which leads me to believe this is a TE issue.
blocking-b2g: leo? → ---
Component: Gaia::Keyboard → Preinstalled B2G Apps
Product: Boot2Gecko → Tech Evangelism
Version: unspecified → Trunk
Blocks: b2g-youtube
No longer blocks: 877024
I'm not sure I agree this is a TE issue.

When pressing the return key, the search box becomes unfocused, and there is then no focused element and keypresses go nowhere.

Regardless of whether YT changed something which exposed the brokenness, it should not be possible for the platform to get into a state where there is no focused input element and the keyboard is visible, keypresses going nowhere.
(In reply to Donovan Preston from comment #9)
> I'm not sure I agree this is a TE issue.

This wasn't present when this was tested previously in previous testing a few months back. It's now present on all builds. YouTube definitely changed something here.

> Regardless of whether YT changed something which exposed the brokenness, it
> should not be possible for the platform to get into a state where there is
> no focused input element and the keyboard is visible, keypresses going
> nowhere.

True, but Leo has already gone to final build here. This wasn't present before and now it is - changed from the server side. At this point, the least risky path forward is to get YouTube to fix this. We could consider filing a separate bug for fixing the underlying platform issue, but it will likely not get into Leo's final build.
I don't think this is a regression. The bug that was fixed, bug 874754, was being unable to open the keyboard even when an input field was focused.

This issue is that the keyboard stays open when an input field is not focused.

Tapping anywhere causes the keyboard to go away, so this does not seem critical, but I will see if YouTube can do blur() on the search field in their onsubmit handler as a workaround.
Okay. I think at this point, let's ask YT to implement a workaround given how late we are in the release. As for the actual underlying bug, I'll move it back to keyboard to get fixed, but we probably can't block on this at this point.
No longer blocks: b2g-youtube
Component: Preinstalled B2G Apps → Gaia::Keyboard
Keywords: regression
Product: Tech Evangelism → Boot2Gecko
Version: Trunk → unspecified
Ok. I think they should be able to implement a simple workaround.
Blocks: 877024
Blocks: b2g-youtube
Not a TE issue per the discussion above.
No longer blocks: b2g-youtube
I tried to reproduce this issue today, and it seems pressing [Enter] button won't make the location changed for search.
If that is the case, we might want to look at if pressing [Enter] would trigger the form submit and make the browser navigate to a new location.
Comment 15 is from a m-c (mozilla-central) build.

Hope to address this in v1.2, so nominate it as koi?
blocking-b2g: --- → koi?
Any update on this? Is this likely to get fixed in 1.2?
Rudy will check this issue.
Assignee: nobody → rlu
blocking-b2g: koi? → koi+
Rudy,

Gentle poke to check if there's been any progress on this bug.
Flags: needinfo?(rlu)
:yxl, does this ring a bell?
Flags: needinfo?(xyuan)
As comment 15 states, we should take a look why pressing [Enter] won't trigger page location change first.
Will ping some Gecko people for help.
Flags: needinfo?(rlu)
I'll take a look at it after lunchtime ;-)
Flags: needinfo?(xyuan)
(In reply to Rudy Lu [:rudyl] from comment #21)
> As comment 15 states, we should take a look why pressing [Enter] won't
> trigger page location change first.
> Will ping some Gecko people for help.

Comparing with the hardware [enter] key events, I found that the keypress event generated by the keyboard API stops YouTube from searching. 

If we don't send keypress event from the softkeyboard, the YouTube app will response to the soft [Enter] key.

YouTube has done some special to the keypress event. We may need YouTube to help with the enter key issue.
(In reply to Yuan Xulei [:yxl] from comment #23)
> (In reply to Rudy Lu [:rudyl] from comment #21)
> > As comment 15 states, we should take a look why pressing [Enter] won't
> > trigger page location change first.
> > Will ping some Gecko people for help.
> 
> Comparing with the hardware [enter] key events, I found that the keypress
> event generated by the keyboard API stops YouTube from searching. 
> 
> If we don't send keypress event from the softkeyboard, the YouTube app will
> response to the soft [Enter] key.
> 
> YouTube has done some special to the keypress event. We may need YouTube to
> help with the enter key issue.

Thanks for the analysis. I will mention it to YouTube and see what they can do.
Yuan,

Thanks for the investigation.

Did we change how we send the keypress event on v1.2?
Since I cannot reproduce the same "page location not changed" issue with v1.1.
Flags: needinfo?(xyuan)
Yes, a little bit. For v1.1 and below, we generate the keypress event in MozKeyboard.js of the IME iframe, while for v1.2 and above, we generate the event in forms.js of the normal app iframe. But the sequence and the parametes of the key events remain the same.
Flags: needinfo?(xyuan)
(In reply to Yuan Xulei [:yxl] from comment #26)
> Yes, a little bit. For v1.1 and below, we generate the keypress event in
> MozKeyboard.js of the IME iframe, while for v1.2 and above, we generate the
> event in forms.js of the normal app iframe. But the sequence and the
> parametes of the key events remain the same.

I don't understand why that cause this difference for keypress event.
Kanru, do you know what might be the root cause?

Thanks.
Flags: needinfo?(kchen)
What's the actual event sequence received by the input element? Do the change between v1.1 and v1.2?
Flags: needinfo?(kchen)
(In reply to Kan-Ru Chen [:kanru] from comment #28)
> What's the actual event sequence received by the input element? Do the
> change between v1.1 and v1.2?

Event sequence(for v1.1 and v1.2): 
keydown -> keypress -> keyup

The related code:
https://github.com/mozilla/mozilla-central/blob/26001cac4dcc8407740e6939e44b772be969bc0c/b2g/chrome/content/forms.js#L513
Hi,

Try to log the key events with this test page,
http://jsfiddle.net/timdream/YDGgk/

The result is here (sorry that these are images since I used Browser app to test this),
http://imgur.com/a/vlzgQ

The only difference I noticed is |evt.key| are different between b2g18 and gecko 27 for each keydown/keypress/keyup events.

Would this make any difference?
(In reply to Rudy Lu [:rudyl] from comment #30)
> Hi,
> 
> Try to log the key events with this test page,
> http://jsfiddle.net/timdream/YDGgk/
> 
> The result is here (sorry that these are images since I used Browser app to
> test this),
> http://imgur.com/a/vlzgQ
> 
> The only difference I noticed is |evt.key| are different between b2g18 and
> gecko 27 for each keydown/keypress/keyup events.
> 
> Would this make any difference?

It shouldn't. evt.key is a new property defined by DOM Level3 Events.
Target Milestone: --- → 1.2 C4(Nov8)
(In reply to Rudy Lu [:rudyl] from comment #30)
> The result is here (sorry that these are images since I used Browser app to
> test this),
> http://imgur.com/a/vlzgQ

Investigating, it looks like the ENTER key event is not the problem here: we send keyCode=0 & charCode=0 on normal keys, which is really bad.
Assignee: rlu → timdream
Status: NEW → ASSIGNED
Tested on Nightly (ua override needed in order to get to m.youtube.com), I was being to solve this problem by sending keyCode when typing alphabets.

Gary, could you review? Rudy is seriously overloaded.

Will test on the phone later.

We need to solve API quirks on these keyCode/charCode things. Will be filing bugs on that.
Attachment #822119 - Flags: review?
Attachment #822119 - Flags: feedback?(rlu)
Comment on attachment 822119 [details]
Github: https://github.com/mozilla-b2g/gaia/pull/13084

OK, this patch does not fix this issue on the phone.

Strangely, Nightly will work with m.youtube.com with or without the patch.
Attachment #822119 - Attachment is obsolete: true
Attachment #822119 - Flags: review?
Attachment #822119 - Flags: feedback?(rlu)
Summarize on what I see here:

-- On master and on device only, when the user pressed enter, the typed search term will be cleaned up and the search will not begin, contrary to the original bug report.
-- I don't see any substantial difference (i.e. events triggered) between the behavior of the vkb and InputMethod API and the actual hardware keyword, after testing on the page I wrote (http://jsfiddle.net/timdream/YDGgk/). Evidently, they work fine with YouTube on Nightly. The differences (keyCode on keyup/down events for normal alphabets and the DOM3 |key| property) are not the cause of this issue.
-- Upon inspecting the page source and try to listen to it's events, I failed to figure out how the page response to Enter keypress.
-- However I could confirm that none of the events we listen in forms.js is being triggered (submit/pagehide/beforeunload).
-- The search page is constructed dynamically (with a new search field) and the page transition is done with |history.pushState()|, but that might not be related.

Given the complexity, I suggest we kick the issue out of v1.2. I also suspect this is a TE issue because human can only figure out how highly compressed JS work to a certain extent.
Assignee: timdream → nobody
Status: ASSIGNED → NEW
blocking-b2g: koi+ → koi?
Summary: [Youtube] The keyboard doesn't hide on pressing ENTER KEY. → [Youtube] Search does not start on pressing ENTER KEY
Note that I am able to produce this bug on B2G/Desktop.

I've also realized we are using different version of forms.js on Nightly (gaia/tools/extensions/keyboard/content/forms.js )and B2G/Desktop & phone (gecko/b2g/chrome/content/forms.js). However I don't see any noticeable difference between two beside composition methods, which is unrelated.
Yeah we sync forms.js manually at the moment. The goal would be to fix this when we remove mozKeyboard altogether.
YouTube Cert Blocker - Being worked on in the dupe.
No longer blocks: 877024
Status: NEW → RESOLVED
blocking-b2g: koi? → koi+
Closed: 11 years ago
Resolution: --- → DUPLICATE
I asked YouTube to provide information about what the code is doing in this case; hopefully they will provide it soon.

Since this bug is older and has more context it would have been nice if the other one was duped to this; but it doesn't matter.
If I open this in Desktop (with Fennec's UA...) there is an onchange event listener on the input directly. I haven't dug into the JS, but it seems like a good candidate for what triggers the search.

Briefly digging into the Gaia keyboard though, it looks like that should be firing...
(In reply to Wesley Johnston (:wesj) from comment #40)
> Briefly digging into the Gaia keyboard though, it looks like that should be
> firing...

Yeah, the problem is, with http://jsfiddle.net/timdream/YDGgk/ I didn't any event, even properties missing (except comment 34 -- however that doesn't fix this bug).
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: