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

PWA not applicable audits are misleading #5558

Closed
ghost opened this issue Jun 26, 2018 · 23 comments
Closed

PWA not applicable audits are misleading #5558

ghost opened this issue Jun 26, 2018 · 23 comments

Comments

@ghost
Copy link

ghost commented Jun 26, 2018

Bug report

Provide the steps to reproduce

  1. Run LH on any webpage (for example on youtube.com).

What is the current behavior?

screenshot_185

(console is count to that)

What is the expected behavior?

No error, viewport is correctly if set dock side of console at bottom or as external window.

Environment Information

V8 6.7.288.46
Related issues

@patrickhulce
Copy link
Collaborator

For others trying to reproduce, this requires that mobile emulation be disabled and you run in DevTools.

More broadly, I think this audit is 100% useless when mobile emulation is disabled anyhow since it's only catching cases where the content isn't sized the same as the viewport on a phone. Perhaps we should mark not applicable when mobile emulation is off?

@paulirish
Copy link
Member

likely resolved with #5893

@exterkamp
Copy link
Member

Have we confirmed this is fixed?

@ghost
Copy link
Author

ghost commented Jan 6, 2019

I think, it's fixed in Chrome 71.0.3578.98,...

@ghost ghost closed this as completed Jan 6, 2019
@ghost
Copy link
Author

ghost commented Feb 26, 2019

Chrome 72.0.3626.119 again that bug

@ghost ghost reopened this Feb 26, 2019
@patrickhulce
Copy link
Collaborator

@Mlocik97-issues the fix for this is not in v3.2.0 which is what is currently in stable Chrome. The fix should be in the next major version of Chrome in Lighthouse v4.0.0-alpha and higher.

@patrickhulce
Copy link
Collaborator

So while it's true that Chrome 72.0.3626.119 never had the fix to begin with and is expected to fail, even in with versions of Lighthouse that have the fix, Chrome will still fail.

The workaround in LH uses emulatedFormFactor but Chrome DevTools is still using the deprecated disableDeviceEmulation https://cs.chromium.org/chromium/src/third_party/blink/renderer/devtools/front_end/audits2/Audits2Controller.js?type=cs&q=disableDeviceEmulation+-f:out+f:audits&sq=package:chromium&g=0&l=242

IMO, we should fix this Chrome-side.

@AubreyHewes
Copy link

The problem seems to be more than just an attached dev tools, it seems to also be applicable when the dev tools are detached.. (if this is already known ignore the following ;-))

(Chrome 73.0.3683.75)

Test via minimal html at https://wn2v4zmv58.codesandbox.io/
Open devtools, detach and then audit...

e.g.

The viewport size is 1454px, whereas window size is 1462px
The viewport size is 1981px, whereas window size is 1989px

The difference (when detached) is always 8px? could it in this case be the scrollbar?

The audit pass description of

The audit passes if window.innerWidth === window.outerWidth.

Seems to not be the actual test/audit?

@jflayhart
Copy link

jflayhart commented Sep 17, 2019

@exterkamp Still a bug for me docking our PWA score when it's actually fine. Running Chrome Version 77.0.3865.75 (Official Build) (64-bit)

Screen Shot 2019-09-17 at 10 23 21 AM

Screen Shot 2019-09-17 at 10 23 50 AM

For all intents and purposes, it should pass: https://developers.google.com/web/tools/lighthouse/audits/content-sized-correctly-for-viewport?utm_source=lighthouse&utm_medium=devtools

@connorjclark
Copy link
Collaborator

connorjclark commented Sep 17, 2019

@jflayhart please provide a URL. I only see a grey circle for that audit in 77 (which is wrong too ...)

@paulirish the connection hand off bug is in 77 (doc you made for reference). The multi-client change fixed that, landed 78.

EDIT: I'm not sure if the original bug here has been fixed, hard to reason about since there's 3-4 bugs dealing with device emulation in DevTools.

@ghost
Copy link
Author

ghost commented Sep 17, 2019

seems it's fixed in Chrome 77.

@connorjclark
Copy link
Collaborator

connorjclark commented Sep 19, 2019

Update - there is a bug in 77 where the content-width (and thus PWA) will always fail. If there is another stable release for 77 (this is out of our hands, as it's not a critical security issue, but I expect there should be one), then there will be an update that disables this audit. For 78+, the root cause was fixed and the PWA category will work fully as expected.

@AndrewBogdanovTSS
Copy link

It's still reproducible for me in Chrome 77, although I can't reproduce it with latest Canary 79 so let's hope it's already resolved and will land to stable soon

@davidwieler
Copy link

This is still an issue in Version 78.0.3904.108

running window.innerWidth === window.outerWidth when dev tools is docked to the side results in false. As an external window or docked to bottom, it returns true but "Content is sized correctly for the viewport" still fails.

Although, it is grey so I'd assume it's not actually counting towards the PWA score..?

@patrickhulce
Copy link
Collaborator

it is grey so I'd assume it's not actually counting towards the PWA score

If it is grey it is not applicable and not affecting your score. Is there a reason you say you're seeing it as an issue @davidwieler?

I'm not able to reproduce any behavior other than notApplicable when run as desktop in Canary 81.0.4041.3, the original root issue here.

@ConradSollitt
Copy link

Found this happening the latest Chrome Build (tested on macOS):

  • Version 87.0.4280.88 (Official Build) (x86_64)

Result:

  • No Error when selecting Mobile for the device type
  • Gray dot with warning (but no details) when selecting Desktop for the device type. Happens if either DevTools are docked or not.
  • Doesn't affect the score though but the gray dot and warning did make me stop and double check my app so I would expect the same for other developers who go through every item.

Not sure if this is related but when docked innerWidth equals document.documentElement.scrollWidth instead of outerWidth:

(window.innerWidth === document.documentElement.scrollWidth) === true
(window.innerWidth === window.outerWidth) === false

However the Lighthouse source is using a different API:
https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/audits/content-width.js

@patrickhulce
Copy link
Collaborator

Gray dot with warning (but no details) when selecting Desktop for the device type. Happens if either DevTools are docked or not.

@ConradSollitt what warning are you seeing? A gray dot means the audit doesn't apply to the situation, it shouldn't have details when this happens.

@ConradSollitt
Copy link

@patrickhulce

Here is a screenshot:

image

The app is a private app I wrote but if you wanted to investigate further I could recreate a basic demo if needed. Here is a quick overview of part of the code that might cause the issue (just speculating/guessing though). The layout works in all tested browsers; basically 100% viewport height and width and a top nav allows for scrolling and panning as it is much wider than the page.

HTML from the page:

<body>
   <nav>
     <div></div>
  </nav>
</body>

CSS from the page:

body, html {
    height: 100vh;
    width: 100vw;
}

nav {
    position: absolute;
    overflow-x: scroll;
    top: 0;
    left: 0;
    right: 0;
    touch-action: pan-x;
    z-index: 10;
}

nav div {
    display: flex;
    width: fit-content;
    margin-left: 20px;
}

@patrickhulce
Copy link
Collaborator

Ah @ConradSollitt that's not a warning, that's the audit saying the "Content is sized correctly for the viewport", the audit's advice is not applicable, and everything is working as expected in Lighthouse.

Is there anything in particular that would've helped clarify this? Warnings in the report are big yellow bold text blocks while not applicable audits are gray.

@ConradSollitt
Copy link

Ah, I see now, thanks @patrickhulce

Two ideas come to my mind initially that could help clarify this (mockups below):

Option 1

A summary legend below PWA title and description and above the audits. This would require the most effort however from UX standpoint I think it would be helpful; and for the UI adding a simple legend look nice and visually flows with the rest of the UI in my opinion.
image

When clicking back to Best Practices I see something similar but when I was focused on PWA updates I didn't notice the Best Practices legend it was already at 100 so I didn't check it for the particular app.
image

Option 2

If this is the only item skipped on desktop then perhaps a brief info message next to the item.
image

In this for a Non-PWA, I see helpful info messages next to the audit failures but not for the skipped item.
image

Original Bug Fixed

Additionally, I would consider the original bug topic to be fixed so this issue could be closed out unless it makes sense to keep open. I've tested on both Mac and Windows now using Chrome 87 and it works for Mobile when DevTools is either docked or opened in a separate window:

image

@patrickhulce
Copy link
Collaborator

Thanks for suggestions @ConradSollitt we'll leave this open and let it track better UX around this issue since several people have been tripped up by it here.

@patrickhulce patrickhulce changed the title [BUG] viewport size when console is opened PWA not applicable audits are misleading Dec 21, 2020
@ConradSollitt
Copy link

Thanks for your help @patrickhulce and great work on Lighthouse! I've used it a lot to help improve the speed of sites I work on and learn about new performance issues that I might not come across without it.

@adrianaixba
Copy link
Collaborator

As per Chrome’s updated Installability Criteria, Lighthouse will be deprecating the PWA category in the next upcoming release. For future PWA testing, users will be directed to use the updated PWA documentation. Marking this as closed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants