fix: favicon with color pref crashes firefox (#5977) (#5979)

This commit is contained in:
Ross Wollman 2021-03-29 14:53:18 -07:00 committed by GitHub
parent f1c0d09765
commit 6d6f802e5a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 73 additions and 4 deletions

View File

@ -1,2 +1,2 @@
1238
Changed: lushnikov@chromium.org Wed 17 Mar 2021 12:44:48 AM PDT
1239
Changed: ross.wollman@gmail.com Mon Mar 29 00:27:24 PDT 2021

View File

@ -596,7 +596,7 @@ index afa1eee3a6107067be52bf635e94be4271facee0..8d3e7bca533da4e55cc43843de552a12
* This attempts to save any applicable layout history state (like
* scroll position) in the nsISHEntry. This is normally done
diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp
index 48af2c30f3604a94e6bc1da7848aed3c8ac11c1e..f9c1df0b36b3c9342b81b4c2c7bc7aeca2461f0c 100644
index 48af2c30f3604a94e6bc1da7848aed3c8ac11c1e..17df24bfdd2d9752686a46b6bb0afc575b091fcb 100644
--- a/dom/base/Document.cpp
+++ b/dom/base/Document.cpp
@@ -3490,6 +3490,9 @@ void Document::SendToConsole(nsCOMArray<nsISecurityConsoleMessage>& aMessages) {
@ -638,7 +638,7 @@ index 48af2c30f3604a94e6bc1da7848aed3c8ac11c1e..f9c1df0b36b3c9342b81b4c2c7bc7aec
IgnoreRFP aIgnoreRFP) const {
+ auto* docShell = static_cast<nsDocShell*>(GetDocShell());
+ nsIDocShell::ColorSchemeOverride colorScheme;
+ if (docShell->GetColorSchemeOverride(&colorScheme) == NS_OK &&
+ if (docShell && docShell->GetColorSchemeOverride(&colorScheme) == NS_OK &&
+ colorScheme != nsIDocShell::COLOR_SCHEME_OVERRIDE_NONE) {
+ switch (colorScheme) {
+ case nsIDocShell::COLOR_SCHEME_OVERRIDE_LIGHT:

View File

@ -0,0 +1,8 @@
<svg viewBox="0 0 32 32" xmlns="http://www.w3.org/2000/svg">
<style>
@media (prefers-color-scheme: dark) {
* { background-color: blue; }
}
</style>
<circle fill="red" cx="16" cy="16" r="12" />
</svg>

After

Width:  |  Height:  |  Size: 220 B

61
test/favicon.spec.ts Normal file
View File

@ -0,0 +1,61 @@
/**
* Copyright 2018 Google Inc. All rights reserved.
* Modifications copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import path from 'path';
import { it } from './fixtures';
it('should load svg favicon with prefer-color-scheme', (test, {browserName, browserChannel, headful}) => {
test.fixme(browserName === 'firefox', 'firefox hard crashes in both headless and headful');
test.skip(!headful && browserName !== 'firefox', 'headless browsers, except firefox, do not request favicons');
test.skip(headful && browserName === 'webkit' && !browserChannel, 'playwright headful webkit does not have a favicon feature');
}, async ({page, server}) => {
// Browsers aggresively cache favicons, so force bust with the
// `d` parameter to make iterating on this test more predictable and isolated.
const favicon = `/favicon.svg?d=${Date.now()}`;
server.setRoute(favicon, (req, res) => {
server.serveFile(req, res, path.join(__dirname, 'assets', 'media-query-prefers-color-scheme.svg'));
});
server.setRoute('/page.html', (_, res) => {
res.end(`
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<link rel="icon" type="image/svg+xml" href="${favicon}">
<title>SVG Favicon Test</title>
</head>
<body>
favicons
</body>
</html>
`);
});
await Promise.all([
server.waitForRequest(favicon),
page.goto(server.PREFIX + '/page.html'),
]);
// Add artificial delay since, just because we saw the request for the favicon,
// it does not mean the browser has processed it. There's not a great way to
// hook into something like "favicon is fully displayed" event, so hopefully
// 500ms is enough, but not too much!
await page.waitForTimeout(500);
// Text still being around ensures we haven't actually lost our browser to a crash.
await page.waitForSelector('text=favicons');
});