Issue 434903002: Make webkitCancelFullScreen() an alias of webkitExitFullscreen()

Issue 434903002: Make webkitCancelFullScreen() an alias of webkitExitFullscreen() (Closed)

Created:
6 years, 4 months ago by philipj_slow

Modified:
6 years, 4 months ago

Reviewers:
falken, adamk

CC:
arv+blink, blink-reviews, blink-reviews-dom_chromium.org, Inactive, dglazkov+blink, eae+blinkwatch, rwlbuis, sof

Base URL:
svn://svn.chromium.org/blink/trunk

Project:
blink

Visibility:
Public.

More Reviews

Description

Make webkitCancelFullScreen() an alias of webkitExitFullscreen() It appears to be an accident of history that webkitCancelFullScreen() behaves differently from webkitExitFullscreen(), as it was implemented before the concept of a fullscreen element stack. In Gecko, mozCancelFullScreen() pops only one element from the stack. In the spec, "fully exit fullscreen" is now reserved for user escape and page navigation, so any other instances should be changed where possible. There was no test coverage for the distinction, and it's very unlikely that Web content could meaningfully depend on it. BUG=399556 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179656

Patch Set 1 #

Patch Set 2 : test #

Total comments: 3

Patch Set 3 : polish test #

Created: 6 years, 4 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+54 lines, -12 lines)			Patch
A	LayoutTests/fullscreen/full-screen-cancel-nested.html	View	1 2	1 chunk	+44 lines, -0 lines	0 comments	Download
A +	LayoutTests/fullscreen/full-screen-cancel-nested-expected.txt	View	1 2	1 chunk	+5 lines, -1 line	0 comments	Download
M	Source/core/dom/DocumentFullscreen.h	View		1 chunk	+0 lines, -1 line	0 comments	Download
M	Source/core/dom/DocumentFullscreen.cpp	View		1 chunk	+0 lines, -5 lines	0 comments	Download
M	Source/core/dom/DocumentFullscreen.idl	View		1 chunk	+1 line, -1 line	0 comments	Download
M	Source/web/ChromeClientImpl.cpp	View		2 chunks	+2 lines, -2 lines	0 comments	Download
M	Source/web/FrameLoaderClientImpl.cpp	View		2 chunks	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 13 (0 generated)

Expand Messages | Collapse Messages

falken

On 2014/08/01 09:43:50, philipj wrote: > PTAL If I understand correctly, webkitCancelFullScreen used to pop ...

6 years, 4 months ago (2014-08-05 03:42:17 UTC) #2

philipj_slow

On 2014/08/05 03:42:17, falken wrote: > On 2014/08/01 09:43:50, philipj wrote: > > PTAL > ...

6 years, 4 months ago (2014-08-05 19:54:49 UTC) #4

falken

Thanks! lgtm with a few nits https://codereview.chromium.org/434903002/diff/20001/LayoutTests/fullscreen/full-screen-cancel-nested.html File LayoutTests/fullscreen/full-screen-cancel-nested.html (right): https://codereview.chromium.org/434903002/diff/20001/LayoutTests/fullscreen/full-screen-cancel-nested.html#newcode1 LayoutTests/fullscreen/full-screen-cancel-nested.html:1: <body> nit: add ...

6 years, 4 months ago (2014-08-06 04:18:30 UTC) #5

philipj_slow

I switched to div instead of body to match e.g. full-screen-remove.html, but otherwise followed orders ...

6 years, 4 months ago (2014-08-06 08:45:23 UTC) #7

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/philipj@opera.com/434903002/40001

6 years, 4 months ago (2014-08-06 20:32:59 UTC) #12

Message was sent while issue was closed.

Change committed as 179656

Expand Messages | Collapse Messages