Skip to content

Commit

Permalink
[dart:html] Fix requestFullscreen bindings and type
Browse files Browse the repository at this point in the history
Closes #25324

requestFullscreen returns a Promise and takes in an options parameter.
It also can be accessed either via `requestFullscreen` or
`webkitFullscreen` (only necessary for Safari). The bindings should be
updated to reflect this behavior.

Change-Id: I9401b6a1c8b3f9ac370aad8caac8295e0ee358b8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/229381
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
  • Loading branch information
srujzs authored and Commit Bot committed Jan 25, 2022
1 parent e72363c commit 4c64181
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 16 deletions.
49 changes: 34 additions & 15 deletions sdk/lib/html/dart2js/html_dart2js.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13989,6 +13989,40 @@ class Element extends Node

int get scrollWidth => JS<num>('num', '#.scrollWidth', this).round();

/**
* Displays this element fullscreen.
*
* ## Other resources
*
* * [Fullscreen
* API](https://developer.mozilla.org/en-US/docs/Web/API/Fullscreen_API)
* from MDN.
* * [Fullscreen specification](http://www.w3.org/TR/fullscreen/) from W3C.
*/
@SupportedBrowser(SupportedBrowser.CHROME)
@SupportedBrowser(SupportedBrowser.SAFARI)

This comment has been minimized.

Copy link
@mnordine

mnordine Jan 25, 2022

Contributor

@srujzs Why not Firefox? Or am I misunderstanding? Shouldn't you be doing a check to see if requestFullscreen exists, instead of relying on a prefixed function name that's only supported in 2 browsers?

https://fullscreen.spec.whatwg.org/#ref-for-dom-element-requestfullscreen%E2%91%A0

This comment has been minimized.

Copy link
@mnordine

mnordine Jan 25, 2022

Contributor

Or perhaps Firefox does indeed work with webkitRequestFullscreen()?

This comment has been minimized.

Copy link
@mnordine

mnordine Jan 25, 2022

Contributor

Update: I just checked and it does not.

This comment has been minimized.

Copy link
@srujzs

srujzs Jan 25, 2022

Author Contributor

It is indeed supported on Firefox (requestFullscreen works). This is just a stale annotation that should have been copied over when I copied over the documentation. I'll update it in a follow-up CL.

This comment has been minimized.

Copy link
@srujzs

srujzs Jan 25, 2022

Author Contributor

Long-term, we should probably get rid of these annotations. They're likely to be out-of-date unless we look up the MDN every time and keep that up-to-date.

This comment has been minimized.

Copy link
@mnordine

mnordine Jan 25, 2022

Contributor

It is indeed supported on Firefox (requestFullscreen works)

Sorry, my bad. I missed the requestFullscreen in your patch.

Future<void> requestFullscreen([Map? options]) {
var retValue;
if (options != null) {
retValue = JS(
'',
'(#.requestFullscreen||#.webkitRequestFullscreen).call(#, #)',
this,
this,
this,
convertDartToNative_Dictionary(options));
} else {
retValue = JS(
'',
'(#.requestFullscreen||#.webkitRequestFullscreen).call(#)',
this,
this,
this);
}
if (retValue != null) return promiseToFuture(retValue);
return Future<void>.value();
}

// To suppress missing implicit constructor warnings.
factory Element._() {
throw new UnsupportedError("Not supported");
Expand Down Expand Up @@ -14896,21 +14930,6 @@ class Element extends Node

void setPointerCapture(int pointerId) native;

@JSName('webkitRequestFullscreen')
/**
* Displays this element fullscreen.
*
* ## Other resources
*
* * [Fullscreen
* API](https://developer.mozilla.org/en-US/docs/Web/API/Fullscreen_API)
* from MDN.
* * [Fullscreen specification](http://www.w3.org/TR/fullscreen/) from W3C.
*/
@SupportedBrowser(SupportedBrowser.CHROME)
@SupportedBrowser(SupportedBrowser.SAFARI)
void requestFullscreen() native;

// From ChildNode

void after(Object nodes) native;
Expand Down
16 changes: 16 additions & 0 deletions tests/lib/html/request_fullscreen_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:html';

void main() async {
var documentElement = document.documentElement!;

// `requestFullscreen` requires user interaction to succeed, so this just
// tests that the bindings and type work.
await documentElement.requestFullscreen().catchError((_) {});
// Try it with an options argument.
await documentElement
.requestFullscreen({'navigationUI': 'show'}).catchError((_) {});
}
16 changes: 16 additions & 0 deletions tests/lib_2/html/request_fullscreen_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:html';

void main() async {
var documentElement = document.documentElement!;

// `requestFullscreen` requires user interaction to succeed, so this just
// tests that the bindings and type work.
await documentElement.requestFullscreen().catchError((_) {});
// Try it with an options argument.
await documentElement
.requestFullscreen({'navigationUI': 'show'}).catchError((_) {});
}
4 changes: 3 additions & 1 deletion tools/dom/idl/dart/dart.idl
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,10 @@ interface AudioScheduledSourceNode {

[DartSupplemental]
interface Element : Node {
// Remove operation requestFullscreen only use webKitRequestFullscreen.
// Use template implementation that looks for both the non-prefixed and
// prefixed `requestFullscreen` instead.
[DartSuppress] void requestFullscreen();
[DartSuppress] void webkitRequestFullscreen();
// setAttribute and setAttributeNS can take in non-string values that are
// then converted to strings.
[DartSuppress] void setAttribute(DOMString name, DOMString value);
Expand Down
34 changes: 34 additions & 0 deletions tools/dom/templates/html/impl/impl_Element.darttemplate
Original file line number Diff line number Diff line change
Expand Up @@ -1584,6 +1584,40 @@ $endif

int get scrollWidth => JS<num>('num', '#.scrollWidth', this).round();

/**
* Displays this element fullscreen.
*
* ## Other resources
*
* * [Fullscreen
* API](https://developer.mozilla.org/en-US/docs/Web/API/Fullscreen_API)
* from MDN.
* * [Fullscreen specification](http://www.w3.org/TR/fullscreen/) from W3C.
*/
@SupportedBrowser(SupportedBrowser.CHROME)
@SupportedBrowser(SupportedBrowser.SAFARI)
Future<void> requestFullscreen([Map? options]) {
var retValue;
if (options != null) {
retValue = JS(
'',
'(#.requestFullscreen||#.webkitRequestFullscreen).call(#, #)',
this,
this,
this,
convertDartToNative_Dictionary(options));
} else {
retValue = JS(
'',
'(#.requestFullscreen||#.webkitRequestFullscreen).call(#)',
this,
this,
this);
}
if (retValue != null) return promiseToFuture(retValue);
return Future<void>.value();
}

$!MEMBERS
}

Expand Down

0 comments on commit 4c64181

Please sign in to comment.