Closed Bug 1232345 Opened 6 years ago Closed 6 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(feature-b2g:2.5+, b2g-v2.5 fixed, b2g-master fixed)

VERIFIED FIXED
2.6 S4 - 1/1
feature-b2g 2.5+
Tracking Status
b2g-v2.5 --- fixed
b2g-master --- fixed

People

(Reporter: jj.evelyn, Assigned: lchang)

References

Details

(Whiteboard: [ft:conndevices][partner-blocker][partner-cherry-pick])

User Story

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

Attachments

(2 files)

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.
As a user, I want to preview Websites from Marketplace.
Assignee: nobody → lchang
Blocks: 1211400
No longer blocks: TV_Marketplace_2.5
No longer depends on: 1217771
User Story: (updated)
Blocks: 1217771
Blocks: TV_P1
feature-b2g: --- → 2.5+
Depends on: 1233629
Depends on: 1233682
Blocks: 1234163
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".
Attachment #8699872 - Flags: review?(ehung)
QA Whiteboard: [COM=TV::Marketplace]
Status: NEW → ASSIGNED
Target Milestone: --- → 2.6 S4 - 1/1
Comment on attachment 8699872 [details] [review]
[gaia] luke-chang:1232345_tv_preview_website > mozilla-b2g:master

r+ with comments addressed. Thanks!
Attachment #8699872 - Flags: review?(ehung) → review+
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.
Attachment #8699872 - Flags: approval-gaia-v2.5?
Comment on attachment 8699872 [details] [review]
[gaia] luke-chang:1232345_tv_preview_website > mozilla-b2g:master

Approve for TV 2.5
Attachment #8699872 - Flags: approval-gaia-v2.5? → approval-gaia-v2.5+
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 :)
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.
> 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 :)
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.