Skip to content

Commit

Permalink
colorDepth returns '0' on detached frame
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=265021

Reviewed by Simon Fraser.

The specification requires 24 as a fallback rather than 0:
https://drafts.csswg.org/cssom-view/#dom-screen-colordepth

Always returning 24 is tempting, but maybe not correct long term.

This synchronizes web-platforms-tests for css/cssom-view with:
web-platform-tests/wpt@4647677

* LayoutTests/http/tests/dom/same-origin-detached-window-properties-expected.txt:
* LayoutTests/http/tests/dom/same-origin-detached-window-properties.html:
* LayoutTests/imported/w3c/resources/resource-files.json:
* LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/screen-detached-frame-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/screen-detached-frame.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/w3c-import.log:
* LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/window-scrollBy-display-change-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/window-scrollBy-display-change-ref.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/window-scrollBy-display-change.html: Added.
* Source/WebCore/page/Screen.cpp:
(WebCore::Screen::colorDepth const):

Canonical link: https://commits.webkit.org/270952@main
  • Loading branch information
annevk committed Nov 19, 2023
1 parent 74ae3ea commit 34925ea
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ PASS w.location.reload('') did not throw exception.
PASS !!w.screen is true
PASS w.screen.height is 0
PASS w.screen.width is 0
PASS w.screen.colorDepth is 0
PASS w.screen.pixelDepth is 0
PASS w.screen.colorDepth is 24
PASS w.screen.pixelDepth is 24
PASS w.screen.availLeft is 0
PASS w.screen.availTop is 0
PASS w.screen.availHeight is 0
Expand Down Expand Up @@ -94,8 +94,8 @@ PASS w.location.reload('') did not throw exception.
PASS !!w.screen is true
PASS w.screen.height is 0
PASS w.screen.width is 0
PASS w.screen.colorDepth is 0
PASS w.screen.pixelDepth is 0
PASS w.screen.colorDepth is 24
PASS w.screen.pixelDepth is 24
PASS w.screen.availLeft is 0
PASS w.screen.availTop is 0
PASS w.screen.availHeight is 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
if (w.screen) {
shouldBe("w.screen.height", "0");
shouldBe("w.screen.width", "0");
shouldBe("w.screen.colorDepth", "0");
shouldBe("w.screen.pixelDepth", "0");
shouldBe("w.screen.colorDepth", "24");
shouldBe("w.screen.pixelDepth", "24");
shouldBe("w.screen.availLeft", "0");
shouldBe("w.screen.availTop", "0");
shouldBe("w.screen.availHeight", "0");
Expand Down
1 change: 1 addition & 0 deletions LayoutTests/imported/w3c/resources/resource-files.json
Original file line number Diff line number Diff line change
Expand Up @@ -4492,6 +4492,7 @@
"web-platform-tests/css/cssom-view/scrollTop-display-change-ref.html",
"web-platform-tests/css/cssom-view/scrollingElement-quirks-dynamic-001-ref.html",
"web-platform-tests/css/cssom-view/scrollingElement-quirks-dynamic-002-ref.html",
"web-platform-tests/css/cssom-view/window-scrollBy-display-change-ref.html",
"web-platform-tests/css/cssom/CSSStyleSheet-constructable-concat-ref.html",
"web-platform-tests/css/cssom/HTMLLinkElement-disabled-alternate-ref.html",
"web-platform-tests/css/cssom/insertRule-from-script-ref.html",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

PASS Window.screen on detached frame

Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<!doctype html>
<meta charset="utf-8">
<link rel="help" href="https://drafts.csswg.org/cssom-view/#dom-window-screen">
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1858977">
<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
<link rel="author" href="https://mozilla.org" title="Mozilla">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<iframe></iframe>
<script>
onload = function() {
test(() => {
let frame = document.querySelector("iframe");
let win = frame.contentWindow;
frame.remove();
assert_true(!!win.screen, "Window.screen should be available");
for (let prop of ["top", "left", "width", "height"]) {
let availProp = "avail" + prop[0].toUpperCase() + prop.substr(1);
if (prop == "width" || prop == "height") {
assert_true(prop in win.screen, prop + "must be implemented per spec")
assert_true(availProp in win.screen, availProp + "must be implemented per spec")
}
if (prop in win.screen) {
assert_equals(win.screen[prop], 0, prop);
}
if (availProp in win.screen) {
assert_equals(win.screen[availProp], 0, availProp);
}
}

// https://drafts.csswg.org/cssom-view/#dom-screen-colordepth
// If the user agent does not know the color depth or does not want to
// return it for privacy considerations, it should return 24.
for (let prop of ["colorDepth", "pixelDepth"]) {
assert_equals(win.screen[prop], 24, prop);
}
}, "Window.screen on detached frame");
};
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ List of files:
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/pt-to-px-width.html
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/range-bounding-client-rect-with-display-contents.html
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/resize-event-on-initial-layout.html
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/screen-detached-frame.html
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/screenLeftTop.html
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scroll-back-to-initial-position.html
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/scroll-behavior-default-css.html
Expand Down Expand Up @@ -183,3 +184,6 @@ List of files:
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/window-screen-height.html
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/window-screen-width-immutable.html
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/window-screen-width.html
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/window-scrollBy-display-change-expected.html
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/window-scrollBy-display-change-ref.html
/LayoutTests/imported/w3c/web-platform-tests/css/cssom-view/window-scrollBy-display-change.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
</head>
<body>
<div style="width: 100px; height: 100px; background: green;"></div>
</body>
</html>

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
</head>
<body>
<div style="width: 100px; height: 100px; background: green;"></div>
</body>
</html>

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>Verify that scrolling the root does actually do a visual scroll</title>
<link rel="help" href="https://www.w3.org/TR/cssom-view-1/#dom-window-scrollby">
<link rel="match" href="window-scrollBy-display-change-ref.html">
</head>
<body style="overflow: hidden" onload="window.scrollBy(0, 2000);">
<div style="height: 2000px"></div>
<div style="width: 100px; height: 100px; background: green;"></div>
<div style="height: 2000px"></div>
</body>
</html>

2 changes: 1 addition & 1 deletion Source/WebCore/page/Screen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ unsigned Screen::colorDepth() const
{
RefPtr frame = this->frame();
if (!frame)
return 0;
return 24;
if (frame->settings().webAPIStatisticsEnabled())
ResourceLoadObserver::shared().logScreenAPIAccessed(*frame->document(), ScreenAPIsAccessed::ColorDepth);
return static_cast<unsigned>(screenDepth(frame->view()));
Expand Down

0 comments on commit 34925ea

Please sign in to comment.