Full-screen button is missing in 47 Beta 6

VERIFIED FIXED in Firefox 47

Status

()

defect
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: sflorean, Assigned: xidorn)

Tracking

(Blocks 1 bug, {regression})

47 Branch
mozilla49
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 unaffected, firefox47+ verified, firefox48 verified, firefox49 verified)

Details

Attachments

(1 attachment)

Reporter

Description

3 years ago
Environment: 
Device: Asus Transformer Pad (Android 4.2.1);
Build: Beta 47.0b6;

Steps to reproduce:
1. Go to http://people.mozilla.com/~nhirata/html_tp/elephants-dream.webm;
2. Tap the video to show the video controls.

Expected result:
Full-screen button is displayed near speaker button.

Actual result:
Full-screen button is missing.
Ray, can you investigate this?
Flags: needinfo?(ralin)
Tracked for Fx47 since it's a recent regression.

Comment 4

3 years ago
(In reply to :Margaret Leibovic from comment #1)
> Ray, can you investigate this?
No problem, I'll see what I can do.


Thank you Tim
It would really help if range provided, since mozregression seems not support fennec beta.
Flags: needinfo?(ralin)
Reporter

Comment 5

3 years ago
Regression:
On 47 Beta 4 - full-screen button is displayed
On 47 Beta 6 - full-screen button is not displayed

Pushlog between the two betas: https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?fromchange=FENNEC_47_0b4_BUILD1&tochange=FENNEC_47_0b6_BUILD1

Looking at the pushlog I think that Bug 1268750 might be the cause of this issue.
The bug # on the commit message is incorrect. The correct one is bug 1268749.

We would need to figure out where the unprefixed Fullscreen API is used in the front-end code and revert it, on beta channel.

Updated

3 years ago
Assignee: nobody → ralin

Comment 7

3 years ago
quick test about prefs: https://reviewboard.mozilla.org/r/49963/diff/3#3

Firefox beta on LG G2, manually toggle "full-screen-api.unprefix.enabled" to "true" and fullscreen button is back.

Steps:
1. Download firefox beta from google play
2. Go to http://people.mozilla.com/~nhirata/html_tp/elephants-dream.webm and found full screen button is missing
3. Open new tab to about:config, and toggle "full-screen-api.unprefix.enabled" to value "true"
4. Switch to video tab and reload the page.... then full screen button is back
---- reconfirm ----
5. Toggle "full-screen-api.unprefix.enabled" to "false"
6. Switch back to video tab and reload. Now full screen button is missing again...

Comment 8

3 years ago
Updates:

video control ui changed from prefix api to unpreifx:
https://hg.mozilla.org/releases/mozilla-beta/diff/55ecf767b62d/toolkit/content/widgets/videocontrols.xml#l1.44

It is clear that we moved to unprefix api on front end. As line 44 in videocontrols.xml, if .fullscreenEnabled is undefined(full-screen-api.unprefix.enabled = false), the full screen button fallback to like as audioOnly. I'm not sure how to deal with this regression, it seems more followup patches are related to converting to unprefixed full screen api[0].

Margaret, is anything I could do to fix it?

[0] https://hg.mozilla.org/mozilla-central/filelog/f3f2fa1d7eed5a8262f6401ef18ff8117a3ce43e/dom/webidl/Document.webidl
Flags: needinfo?(margaret.leibovic)
Assignee

Comment 9

3 years ago
I'll take this.
Assignee: ralin → bugzilla
Component: Audio/Video → Video/Audio Controls
Flags: needinfo?(margaret.leibovic)
OS: Android → All
Product: Firefox for Android → Toolkit
Hardware: ARM → All
Assignee

Comment 10

3 years ago
So it seems video document doesn't have chrome permission, and thus unprefixed fullscreen api is not exposed if it is disabled. I'd revert the usage there back to prefixed API, and file another bug for using unprefixed API.
Assignee

Comment 12

3 years ago
The patch is basically just a revert of the toolkit part in bug 743198 part 8.
Attachment #8754142 - Flags: review?(dolske) → review+
Comment on attachment 8754142 [details]
MozReview Request: Bug 1273468 - Revert video controls to use prefixed Fullscreen API again. r?dolske

https://reviewboard.mozilla.org/r/53766/#review50486

rs=me for basically backing out the changes in bug 743198.

What's the plan for going back to the unprefixed version? Do we expect to land this only on beta of fix things soon, or is this a longer term thing?
Assignee

Updated

3 years ago
Blocks: 1274098
Assignee

Comment 14

3 years ago
(In reply to Justin Dolske [:Dolske] from comment #13)
> What's the plan for going back to the unprefixed version? Do we expect to
> land this only on beta of fix things soon, or is this a longer term thing?

Filed bug 1274098. It would also need to land on 48 and 49, and probably stay there as long as we still have the separate pref for unprefixed Fullscreen API.
Assignee

Comment 16

3 years ago
Comment on attachment 8754142 [details]
MozReview Request: Bug 1273468 - Revert video controls to use prefixed Fullscreen API again. r?dolske

Approval Request Comment
[Feature/regressing bug #]: bug 1268749
[User impact if declined]: fullscreen won't work properly for video documents when unprefixed Fullscreen API is disabled (by default for release versions)
[Describe test coverage new/current, TreeHerder]: looks like no coverage currently
[Risks and why]: low risk, it is just a revert of toolkit part in bug 743198, makes video documents to use prefixed Fullscreen API again
[String/UUID change made/needed]: n/a
Attachment #8754142 - Flags: approval-mozilla-beta?
Attachment #8754142 - Flags: approval-mozilla-aurora?
Assignee

Comment 17

3 years ago
hmmm, wait... I suppose it would break test_videocontrols.html on beta...
Assignee

Comment 18

3 years ago
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #17)
> hmmm, wait... I suppose it would break test_videocontrols.html on beta...

Hmmmm, it won't... test_videocontrols.html is run as a mochitest-chrome, which does have chrome permission and thus always have access to unprefixed Fullscreen API.

I think we should make that test only have content permission so that we can catch regressions which only present in content.
Assignee

Comment 19

3 years ago
Looks like test_videocontrols.html was moved from mochitest to mochitest-chrome in bug 694696, and the reason was described in bug 694696 comment 18. Not sure what should we do here...
Assignee

Comment 20

3 years ago
(In reply to Xidorn Quan [:xidorn] (UTC+10) from comment #16)
> [User impact if declined]: fullscreen won't work properly for video
> documents when unprefixed Fullscreen API is disabled (by default for release
> versions)

Not only video documents. All video controls are affected.
Assignee

Comment 23

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/33509142de5fe1f6357006c391de85d7a6f11943
Bug 1273468 - Revert video controls to use prefixed Fullscreen API again. r=dolske
Assignee

Updated

3 years ago
Blocks: 1274104

Comment 25

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/33509142de5f
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment on attachment 8754142 [details]
MozReview Request: Bug 1273468 - Revert video controls to use prefixed Fullscreen API again. r?dolske

Recent regression, Aurora48+, Beta47+
Attachment #8754142 - Flags: approval-mozilla-beta?
Attachment #8754142 - Flags: approval-mozilla-beta+
Attachment #8754142 - Flags: approval-mozilla-aurora?
Attachment #8754142 - Flags: approval-mozilla-aurora+
Hi Sorina, could you please verify this issue is fixed as expected on a latest Nightly/Aurora/Beta build? Thanks!
Flags: needinfo?(sorina.florean)
Assignee

Comment 28

3 years ago
To verify the fix, you need to turn off unprefixed fullscreen api via setting full-screen-api.unprefix.enabled to false. With the unprefixed api enabled, this bug would not appear.
I'm hitting conflicts uplifting this to aurora:
$ hg graft -er 33509142de5f
grafting 343977:33509142de5f "Bug 1273468 - Revert video controls to use prefixed Fullscreen API again. r=dolske"
merging toolkit/content/TopLevelVideoDocument.js
merging toolkit/content/tests/widgets/test_videocontrols.html
merging toolkit/content/widgets/videocontrols.xml
warning: conflicts while merging toolkit/content/tests/widgets/test_videocontrols.html! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)
Flags: needinfo?(bugzilla)
Reporter

Comment 31

3 years ago
Hi Ritu,

Tested using:
Asus Transformer Pad (Android 4.2.1) 
LG G4 (Android 5.1)
Nexus 5 (Android 6.0.1)

After May 18th, on https://ftp.mozilla.org/pub/mobile/nightly/2016/05/ aren't builds available.
Build: Nightly 49.0a1 (2016-05-18), on phone and tablet: 
-Full-screen-api.unprefix.enabled to false-> full-screen button is missing;
-Full-screen-api.unprefix.enabled to true-> full-screen button is displayed;
 

Build: Aurora 48.0a2 (2016-05-22), on phone: 
-Full-screen-api.unprefix.enabled to false-> full-screen button is missing;
-Full-screen-api.unprefix.enabled to true-> full-screen button is missing;  

Build: Aurora 48.0a2 (2016-05-22), on tablet: 
-Full-screen-api.unprefix.enabled to false-> full-screen button is displayed;
-Full-screen-api.unprefix.enabled to true-> full-screen button is displayed;

Comment 32

3 years ago
Hi, :sorina

Is missing button on Aurora because of bug 1267935?
Reporter

Comment 33

3 years ago
(In reply to Ray Lin[:ralin] from comment #32)

> Is missing button on Aurora because of bug 1267935?

I think that Bug 1267935 might be the problem, who is only affecting Aurora. Please see that on phone and tablet, the behavior is different.
Flags: needinfo?(sorina.florean)
Status: RESOLVED → VERIFIED
Reporter

Comment 35

3 years ago
On 47 Beta 8 full-screen button is displayed.
Tested using Nexus 5 (Android 6.0.1) and Samsung Galaxy R (Android 2.3.4).

Updated

2 years ago
Depends on: 1332699
You need to log in before you can comment on or make changes to this bug.