1232345 - [TV][2.5] Preview Website in Preview window.

Status: VERIFIED FIXED

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

Description

[Evelyn Hung]

In bug 1225081 we added a PreviewWindow as a child window of marketplace. We should use it to display website content opened by Marketplace as well.

Comment 1

[Evelyn Hung]

As a user, I want to preview Websites from Marketplace.

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] luke-chang:1232345_tv_preview_website > mozilla-b2g:master

Comment 3

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8699872 [details] [review]
[gaia] luke-chang:1232345_tv_preview_website > mozilla-b2g:master

Hi Evelyn,

Could you help review this patch? It contains five commits but only the last one is against this bug -- the others can be ignored.

In this patch I changed the behavior of the BACK button and also fixed a "focus" bug in "app_install_dialogs.js".

Comment 4

[Evelyn Hung]

Comment on attachment 8699872 [details] [review]
[gaia] luke-chang:1232345_tv_preview_website > mozilla-b2g:master

r+ with comments addressed. Thanks!

Comment 5

[Luke Chang [:lchang] (inactive)]

tests passed: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=3d1c163d29d918c3737a1587defd8ad94709ef1c

landed on master: https://github.com/mozilla-b2g/gaia/commit/4742f8102bee9f41f3e107c5d8f020375e76d78f

Comment 6

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8699872 [details] [review]
[gaia] luke-chang:1232345_tv_preview_website > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): TV feature: OWD
[User impact] if declined: feature isn't complete.
[Testing completed]: yes.
[Risk to taking this patch] (and alternatives if risky): low, affect TV only.
[String changes made]: yes.

Comment 7

[Josh Cheng [:josh]]

Comment on attachment 8699872 [details] [review]
[gaia] luke-chang:1232345_tv_preview_website > mozilla-b2g:master

Approve for TV 2.5

Comment 8

[Carsten Book [:Tomcat]]

for 2.5 https://github.com/mozilla-b2g/gaia/commit/b54eddf0e2e8c998041f26d455dd470fcbcd7ba8

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

In this patch you introduced a bad design regarding l10n labels:

BrowserContextMenu is carrying `textRaw` which is resolved at creation time using mozL10n.get which is deprecated.

Instead, you should carry l10nId around and resolve it by assigning data-l10n-id on the element in which the entity is used.

I'm just writing here to encourage avoiding such decisions in the future :)

Comment 10

[Luke Chang [:lchang] (inactive)]

Hi Zibi,

Thanks for your feedback. And yes, I do know it's a bad design. Actually, the context menu in TV, which is rendered by a smart component "smart-modal-dialog", support the “textL10nId” property indeed and will assign the "data-l10n-id" accordingly. However, its former version, which is being used in TV smart system, has a bug that it can't support "textL10nId" and "menuIcon" at the same time. Although it has been fixed in the latest version, its interface has also been changed so we can't apply it to TV smart system easily. Since TV 2.5 will code freeze soon, I intended to avoid too many changes on it and compromised with that bad design.

I promise it'll be fixed in the master branch in the near future. Thanks again.

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

> I promise it'll be fixed in the master branch in the near future. Thanks again.

Cool! Thanks for the response! I understand the constrains :)

Comment 12

[[e-mail]]

Attachment: Screenshot from 2016-02-23 15_56_50.png

Comment 13

[[e-mail]]

Verify this issue on the latest build.
A preview window as a child window is displayed on the upper-right corner of marketplace page.
Please refer to the attachment(Screenshot from 2016-02-23 15_56_50.png)

[Build Info]
simulator 2.6.20160222133739
gaia:
https://github.com/mozilla-b2g/gaia/commit/d9faad5c0f14fcd42f920c9104f0638f46a48b2a
gecko:
https://hg.mozilla.org/mozilla-central/rev/789a12291942763bc1e3a89f97e0b82dc1c9d00b