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

core(doctype): check document.compatMode for quirks mode #12978

Merged
merged 14 commits into from
Apr 26, 2022
Merged

core(doctype): check document.compatMode for quirks mode #12978

merged 14 commits into from
Apr 26, 2022

Conversation

k99sharma
Copy link
Contributor

Summary
This change adds a new check in doctype audit for compatMode of the document. It is needed to check for quirks-mode of the document.
I read about quirks-mode check in this MDN doc: doc

Related Issues/PRs
Issue no: #10030

- new check is added in `doctype` audit for checking `compatMode` of document
- `getDoctype` function in `gatherer` is also altered to return compatMode
- `Doctype` interface is also changed to return `compatMode` as string in `artifacts.d.ts`
@k99sharma k99sharma requested a review from a team as a code owner August 24, 2021 15:41
@k99sharma k99sharma requested review from connorjclark and removed request for a team August 24, 2021 15:41
@google-cla google-cla bot added the cla: yes label Aug 24, 2021
@k99sharma k99sharma changed the title quirks-mode check is added in doctype audit misc(document compatibility check for quirks-mode): added Aug 24, 2021
@k99sharma
Copy link
Contributor Author

k99sharma commented Aug 24, 2021

I changed the doctype audit to check for document quirks-mode. I also changed getDoctype function in gatherer file to return compatMode.
I ran yarn test command to test the files and everything worked out. If you want any changes please let me know.

@@ -17,11 +17,11 @@ type _TaskNode = import('../lighthouse-core/lib/tracehouse/main-thread-tasks.js'
import AuditDetails from './lhr/audit-details'
import Config from './config';
import Gatherer from './gatherer';
import {IcuMessage} from './lhr/i18n';
import { IcuMessage } from './lhr/i18n';
Copy link
Collaborator

Choose a reason for hiding this comment

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

would you revert all of these formatting changes please? makes it hard to tell what the underlying change is :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My IDE automatically converted these I will change them back!

Copy link
Contributor Author

@k99sharma k99sharma Aug 24, 2021

Choose a reason for hiding this comment

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

I was not able to revert it. But I deleted that file and made another commit with the required changes only.

@connorjclark
Copy link
Collaborator

Hey @k99sharma would you be able to fix the merge conflicts?

@paulirish paulirish changed the title misc(document compatibility check for quirks-mode): added core(doctype): report if the page isn in quirks mode Apr 25, 2022
@paulirish
Copy link
Member

Hey @k99sharma would you be able to fix the merge conflicts?

i took care of 'em. and fixed up a few other pieces.

@connorjclark connorjclark changed the title core(doctype): report if the page isn in quirks mode core(doctype): check document.compatMode for quirks mode Apr 26, 2022

// Catch-all for any quirks-mode situations the above checks didn't get.
// https://github.com/GoogleChrome/lighthouse/issues/10030
if (compatMode === 'BackCompat') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm thinking we should just only check this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually... I guess the specific messages are better for diagnosing. nevermind.

@paulirish paulirish merged commit 90775ac into GoogleChrome:master Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants