Continuously calling requestFullscreen() multiple times causes unexpected result

RESOLVED FIXED in Firefox 43

Status

()

Core
DOM
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: xidorn, Assigned: xidorn)

Tracking

(Blocks: 1 bug, {regression})

Trunk
mozilla43
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 unaffected, firefox41 unaffected, firefox42 affected, firefox43 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
If the content calls requestFullscreen() for multiple times in the same event handler, and the fullscreen transition is not disabled, the later fullscreen request would be applied first before we actually toggle the window state.

This is a regression since we have the fullscreen transition.
(Assignee)

Updated

3 years ago
Assignee: nobody → quanxunzhen
(Assignee)

Comment 1

3 years ago
Created attachment 8652716 [details]
MozReview Request: Bug 1198563 part 1 - Encapsulate iterating fullscreen request list code.

Bug 1198563 part 1 - Encapsulate iterating fullscreen request list code.
Attachment #8652716 - Flags: review?(bugs)
(Assignee)

Comment 2

3 years ago
Created attachment 8652717 [details]
MozReview Request: Bug 1198563 part 2 - Do no directly apply fullscreen state when window object reports in fullscreen when there are pending request before.

Bug 1198563 part 2 - Do no directly apply fullscreen state when window object reports in fullscreen when there are pending request before.
Attachment #8652717 - Flags: review?(bugs)
(Assignee)

Updated

3 years ago
status-firefox40: --- → unaffected
status-firefox41: --- → unaffected
status-firefox42: --- → affected

Comment 3

3 years ago
Comment on attachment 8652717 [details]
MozReview Request: Bug 1198563 part 2 - Do no directly apply fullscreen state when window object reports in fullscreen when there are pending request before.

>+  if ((static_cast<nsGlobalWindow*>(rootWin.get())->FullScreen() &&
>+       PendingFullscreenRequestList::Iterator(this).AtEnd()) ||
>+      nsContentUtils::GetRootDocument(this)->IsFullScreenDoc()) {
AtEnd() is a bit odd. Could you add something like IsEmpty() to the iterator?
Attachment #8652717 - Flags: review?(bugs) → review+
(Assignee)

Comment 4

3 years ago
(In reply to Olli Pettay [:smaug] (high review load) from comment #3)
> Comment on attachment 8652717 [details]
> MozReview Request: Bug 1198563 part 2 - Do no directly apply fullscreen
> state when window object reports in fullscreen when there are pending
> request before.
> 
> >+  if ((static_cast<nsGlobalWindow*>(rootWin.get())->FullScreen() &&
> >+       PendingFullscreenRequestList::Iterator(this).AtEnd()) ||
> >+      nsContentUtils::GetRootDocument(this)->IsFullScreenDoc()) {
> AtEnd() is a bit odd. Could you add something like IsEmpty() to the iterator?

I don't think IsEmpty() makes any sense for an iterator... I agree AtEnd() is a bit odd here, but it's probably better to describe it with a comment instead of adding a weird method to the iterator.

Comment 5

3 years ago
Comment on attachment 8652716 [details]
MozReview Request: Bug 1198563 part 1 - Encapsulate iterating fullscreen request list code.

>+ auto lastRequest = PendingFullscreenRequestList::GetLast();
Trying to sell me 'auto' ;)

No no, that doesn't improve readability at all, since the actual type of lastRequest is totally hidden in that context.
So, just keep FullscreenRequest*
Attachment #8652716 - Flags: review?(bugs) → review+

Comment 6

3 years ago
(In reply to Xidorn Quan [:xidorn] (UTC+12) from comment #4)
> I don't think IsEmpty() makes any sense for an iterator... I agree AtEnd()
> is a bit odd here, but it's probably better to describe it with a comment
> instead of adding a weird method to the iterator.
Fine, add a comment.
(Assignee)

Comment 7

3 years ago
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/42175a953992b5cc83a3cdbd6c1b45b52edffe30
changeset:  42175a953992b5cc83a3cdbd6c1b45b52edffe30
user:       Xidorn Quan <quanxunzhen@gmail.com>
date:       Thu Aug 27 11:21:29 2015 +1000
description:
Bug 1198563 part 1 - Encapsulate iterating fullscreen request list code. r=smaug

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/690c350cf96ca008fada30cab9752c1a5d72e7c3
changeset:  690c350cf96ca008fada30cab9752c1a5d72e7c3
user:       Xidorn Quan <quanxunzhen@gmail.com>
date:       Thu Aug 27 11:21:29 2015 +1000
description:
Bug 1198563 part 2 - Do no directly apply fullscreen state when window object reports in fullscreen when there are pending request before. r=smaug
https://hg.mozilla.org/mozilla-central/rev/42175a953992
https://hg.mozilla.org/mozilla-central/rev/690c350cf96c
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
(Assignee)

Updated

3 years ago
Blocks: 1189200
(Assignee)

Updated

3 years ago
No longer blocks: 1189200
Depends on: 1189200
You need to log in before you can comment on or make changes to this bug.