1227537 - [TV][2.5] Failed to navigate back

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::TV::System, defect, P1)

Description

[Cynthia Tang [:cynthiatang]]

Attachment: video

UX spec: 
 - marketplace-tv-151118, page 12

STR:
 1. Launch Marketplace
 2. Press arrow keys to select "Mp3 Download"
 3. Navigated to different pages (Please see the video)
 4. Press "back"

Actual Result: 
 - Failed to navigate back

Expected Result:
 - User can press back until they reach the landing page

Branch Master
Commit 98e460ceef4b84752e3a5dac89e3507d593666e4
Author: Evelyn Hung <[e-mail]>
Date:   Mon Nov 23 18:35:58 2015 +0800

Comment 1

[Ricky Chien [:rickychien] (inactive)]

Interesting!

Video showed when opening a link if it contains <a target='_blank'> or <a target='_new'> ...etc, system will try to open a new window along with an opening animation. 

See https://youtu.be/nxKZVE8La38?t=41

We can prevent user to open new window / tab and display the content inside self iframe or just disable such link to be opened.

Comment 2

[Ricky Chien [:rickychien] (inactive)]

Preview window is a kind of child window. User is able to navigate within child window and click an url link to explore new content. However, there are problems if an url link tries to open content in new window. We have to decide whether we should:

1. open the content with a new child window
2. open the content inside self /original child window
3. just prevent user to open url link (it will be a new marketplace rule to inform developers that webapp is unable to open a new window by <a target='_blank'> or <a target='_new'>...etc)

Tori, could you give us your feedbacks for this? thanks

Comment 3

[Ricky Chien [:rickychien] (inactive)]

Tori, feel free to reach out to me if you have question about comment 2. :)

Comment 4

[Tori Chen [:tori]]

After offline discussion, option 2 (open the content inside self /original child window) seems the best option both for UX and engineering side.

(In reply to Ricky Chien [:rickychien] from comment #2)
> Preview window is a kind of child window. User is able to navigate within
> child window and click an url link to explore new content. However, there
> are problems if an url link tries to open content in new window. We have to
> decide whether we should:
> 
> 1. open the content with a new child window
> 2. open the content inside self /original child window
> 3. just prevent user to open url link (it will be a new marketplace rule to
> inform developers that webapp is unable to open a new window by <a
> target='_blank'> or <a target='_new'>...etc)

Comment 5

[GitHub Autolander Bot]

Attachment: [gaia] rickychien:fail-to-back > mozilla-b2g:master

Comment 6

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8701801 [details] [review]
[gaia] rickychien:fail-to-back > mozilla-b2g:master

Luke, it's a small patch to detect when user clicks an url such as <a target='_blank|_new' href=...> which requests to open a content within new window, we must avoid dispatching 'openwindow' event to create a new instance again.

We skip to invoke createNewWindow() when there is an window.open request happen in preview window.

Comment 7

[Ricky Chien [:rickychien] (inactive)]

After skipping createNewWindow(), we'll try to open the url in default behavior (popup window will be shown in this case)

Comment 8

[Ricky Chien [:rickychien] (inactive)]

After offline discussion with Tori, finally we decide to open content within the original window itself rather than a popup window.

Comment 9

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8701801 [details] [review]
[gaia] rickychien:fail-to-back > mozilla-b2g:master

The patch itself looks good but it only takes care of the "window.open" with its name set to "_blank/_new". We may also need to consider what it should do when the "name" is set to other values. Let's discuss it with UX.

Comment 10

[Ricky Chien [:rickychien] (inactive)]

Finally decision for question mentioned on comment 9 we'll disable and block target="framename" opening to avoid jumping to another window and lose navigation functionality.

Comment 11

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8701801 [details] [review]
[gaia] rickychien:fail-to-back > mozilla-b2g:master

Hey Luke, 

this new year small gift has been updated, please take a look thanks you!

Comment 12

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8701801 [details] [review]
[gaia] rickychien:fail-to-back > mozilla-b2g:master

Thanks for your work.

Comment 13

[Ricky Chien [:rickychien] (inactive)]

Thank you Luke.

Landed in master at https://github.com/mozilla-b2g/gaia/commit/acbcacb80be0cbe7d78d43b5befdd14c65b97324

Comment 14

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8701801 [details] [review]
[gaia] rickychien:fail-to-back > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 1232345
[User impact] if declined: Fail back key navigation on certain page
[Testing completed]: verified on local
[Risk to taking this patch] (and alternatives if risky): impact would happen when user opens preview content in marketplace app.
[String changes made]:

Comment 15

[Josh Cheng [:josh]]

Comment on attachment 8701801 [details] [review]
[gaia] rickychien:fail-to-back > mozilla-b2g:master

Approved for TV 2.5

Comment 16

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/36aff82e414af753e46eb3743c14fb930fbae305