Closed Bug 1205325 Opened 4 years ago Closed 4 years ago

[TV 2.5][Browser] Failed to pin a web page to Home screen

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.5+, b2g-master affected)

RESOLVED FIXED
FxOS-S9 (16Oct)
blocking-b2g 2.5+
Tracking Status
b2g-master --- affected

People

(Reporter: cynthiatang, Assigned: schien)

References

Details

(Whiteboard: [ft:conndevices])

Attachments

(1 file, 2 obsolete files)

STR:
 1. Launch Browser and go to a web page
 2. Move cursor to Start icon and press OK to add web page to bookmark
 3. Press Up and press OK to select the checkbox "Also pin to home screen"
 4. Save it 

Actual: 
 - Failed to pin a web page to Home screen

Expected:
 - User can pin a web page to Home screen
[Blocking Requested - why for this release]:
It should work well.
blocking-b2g: --- → 2.5?
User could not browse other web pages after getting the failure for "Pin to Home"

The log for "Pin to Home":
###################################### BrowserElementCopyPaste.js loaded
############################### browserElementPanningAPZDisabled.js loaded
############################### browserElementPanning.js loaded
######################## BrowserElementChildPreload.js loaded
JavaScript error: app://browser.gaiamobile.org/js/awesomescreen.js, line 1826: TypeError: 'getScreenshot' called on an object that does not implement interface HTMLIFrameElement.

The log for browsing other web page: 
Set intl.ime.nstextinput.enable to true in about:config to fix input.
JavaScript error: http://bcc.com/, line 232: ReferenceError: _gat is not defined
JavaScript error: http://bcc.com/, line 241: ReferenceError: _gat is not defined
JavaScript error: chrome://global/content/BrowserElementChildPreload.js, line 109: NS_ERROR_FAILURE:
(In reply to Cynthia Tang [:cynthiatang] from comment #2)
> User could not browse other web pages after getting the failure for "Pin to
> Home"
> 
> The log for "Pin to Home":
> ###################################### BrowserElementCopyPaste.js loaded
> ############################### browserElementPanningAPZDisabled.js loaded
> ############################### browserElementPanning.js loaded
> ######################## BrowserElementChildPreload.js loaded
> JavaScript error: app://browser.gaiamobile.org/js/awesomescreen.js, line
> 1826: TypeError: 'getScreenshot' called on an object that does not implement
> interface HTMLIFrameElement.
> 
We need to call the |getScreenshot| on a browser-element iframe object instead of calling like a plain javascript function. [1][2]

This reason of this porting issue is that on latest m-c we've WebIDL-ify the Browser API (bug 1044736). In the past, |getScreenshot| is plain javascript function Gecko injected to browser-element iframe. After WebIDL-ify, |getScreenshot| becomes a member function of browser-element.

I have a patch to fix the JS error from |getScreenshot| but it still have issue creating a card on home overlay. I suspect there is some more issue in card manager.

[1] https://github.com/mozilla-b2g/gaia/blob/master/tv_apps/browser/js/awesomescreen.js#L1795
[2] https://github.com/mozilla-b2g/gaia/blob/master/tv_apps/browser/js/awesomescreen.js#L1826
Attached patch [WIP] pin-page-to-home.patch (obsolete) — Splinter Review
This patch contains following fix:
1. can add card for new web page on home screen
2. can open page in new tab while browser app is launched already.
3. can filter by "website" category on home screen

TODO: cannot create tab for displaying opened page

NOTE: new created card cannot display icon correctly, thus I cannot verify if screenshot is captured or not. Need @danhuang's help on this.
Assignee: nobody → schien
Attached patch [WIP] pin-page-to-home.patch v2 (obsolete) — Splinter Review
With this patch I can correctly open pinned page even if browser app is not launched yet. However the code is kinda scary to me. Need to discuss with @rexboy and @yifan.
Attachment #8667840 - Attachment is obsolete: true
Blocks: TV_FxOS2.5
Status: NEW → ASSIGNED
blocking-b2g: 2.5? → 2.5+
Target Milestone: --- → FxOS-S9 (16Oct)
Attachment #8667858 - Attachment is obsolete: true
Attachment #8668260 - Flags: review?(yliao)
Attachment #8668260 - Flags: review?(rexboy)
Comment on attachment 8668260 [details] [review]
[gaia] schien:bug1205325-fix-pin-page-to-home > mozilla-b2g:master

Thank you! Looks good!
Attachment #8668260 - Flags: review?(yliao) → review+
Comment on attachment 8668260 [details] [review]
[gaia] schien:bug1205325-fix-pin-page-to-home > mozilla-b2g:master

Looks good to me but I found the screenshot size made is not big enough. Although the size of screenshot is set to 658 x 336px but the screenshot we got actually is 411 x 210px. I tried to do a little trace but didn't find the reason. We may need to keep track of this issue.
Attachment #8668260 - Flags: review?(rexboy) → review+
Hi Tori, 
does TV browser allow the user to pin the same web page to Home? Thank you.
Flags: needinfo?(tchen)
(In reply to KM Lee [:rexboy] (AWAY 10/3~10/12) from comment #8)
> Comment on attachment 8668260 [details] [review]
> [gaia] schien:bug1205325-fix-pin-page-to-home > mozilla-b2g:master
> 
> Looks good to me but I found the screenshot size made is not big enough.
> Although the size of screenshot is set to 658 x 336px but the screenshot we
> got actually is 411 x 210px. I tried to do a little trace but didn't find
> the reason. We may need to keep track of this issue.

The image size is 1645*840, reported by WebIDE. I found the CSS "background-size" property for that card is set to "33.6rem auto". The background image will fill the entire circle and still get high quality image when I change "background-size" to "cover". I feel this is an issue in CSS instead of in javascript, while we trying to fit a wide-screen image into a rectangle.

I've filed bug 1211349 for the following modification.
(In reply to Cynthia Tang [:cynthiatang] from comment #9)
> Hi Tori, 
> does TV browser allow the user to pin the same web page to Home? Thank you.

Move this ni? to bug 1211354.
Flags: needinfo?(tchen)
https://github.com/mozilla-b2g/gaia/commit/06b8907fdceabdfccde1b23900d21ace5b33252a
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.