Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add favicon test asset #6868

Merged
merged 4 commits into from Feb 11, 2021
Merged

fix: add favicon test asset #6868

merged 4 commits into from Feb 11, 2021

Conversation

mathiasbynens
Copy link
Member

@whimboo
Copy link
Collaborator

whimboo commented Feb 11, 2021

Thanks a lot for adding the favicon! Please note that at least the above mentioned test will now pass in Firefox. I would expect that others might also pass. So itFailsFirefox would have to be removed too.

@@ -29,8 +30,9 @@
<button id=btn9>9</button>
<button id=btn10>10</button>
<script>
window.addEventListener('DOMContentLoaded', () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't around when this test was created, but I imagine it was written this way assuming the favicon warning would be logged before DOMContentLoaded. Now that we have a favicon, we no longer need to try and avoid the error message.

@mathiasbynens
Copy link
Member Author

@whimboo Done! Please mark review+ through GitHub's UI once everything looks okay.

Copy link
Collaborator

@whimboo whimboo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine from my side, but as I'm not a proper reviewer for this repo you might want to seek someone else too.

@mathiasbynens mathiasbynens merged commit a63f53c into main Feb 11, 2021
@mathiasbynens mathiasbynens deleted the favicon branch February 11, 2021 11:54
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 13, 2021
…mote-protocol-reviewers,jdescottes

Temporarily add a favicon to fix Puppeteer tests relying on a proper set of console log messages. It's the same favicon as used in puppeteer/puppeteer#6868 which will land post Puppeteer 7.0.4.

Differential Revision: https://phabricator.services.mozilla.com/D104809
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Feb 16, 2021
…mote-protocol-reviewers,jdescottes

Temporarily add a favicon to fix Puppeteer tests relying on a proper set of console log messages. It's the same favicon as used in puppeteer/puppeteer#6868 which will land post Puppeteer 7.0.4.

Differential Revision: https://phabricator.services.mozilla.com/D104809

UltraBlame original commit: 045d91aa8b2f2921d8bfd0173a8e68c7a3b65cdb
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Feb 16, 2021
…mote-protocol-reviewers,jdescottes

Temporarily add a favicon to fix Puppeteer tests relying on a proper set of console log messages. It's the same favicon as used in puppeteer/puppeteer#6868 which will land post Puppeteer 7.0.4.

Differential Revision: https://phabricator.services.mozilla.com/D104809

UltraBlame original commit: 045d91aa8b2f2921d8bfd0173a8e68c7a3b65cdb
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Feb 16, 2021
…mote-protocol-reviewers,jdescottes

Temporarily add a favicon to fix Puppeteer tests relying on a proper set of console log messages. It's the same favicon as used in puppeteer/puppeteer#6868 which will land post Puppeteer 7.0.4.

Differential Revision: https://phabricator.services.mozilla.com/D104809

UltraBlame original commit: 045d91aa8b2f2921d8bfd0173a8e68c7a3b65cdb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants