New issue
Advanced search Search tips

Issue 690593 link

Starred by 5 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 674593


Participants' hotlists:
Lifecycle

Show other hotlists

Other hotlists containing this issue:
Non-Standard-IDL


Sign in to add a comment

Remove non-standard APIs |webkitHidden| and |webkitVisibilityState| in Document

Project Member Reported by lunalu@chromium.org, Feb 9 2017

Issue description

Tests: https://jsbin.com/cavaxah/edit?html,output
Implemented in: Blink only, not EdgeHTML, Gecko or WebKit
Standard: https://www.w3.org/TR/page-visibility/#sec-document-interface
 

Comment 1 by phistuck@gmail.com, Feb 9 2017

https://www.chromestatus.com/metrics/feature/popularity#PrefixedPageVisibility
At 4%, it will be a while before this is removed.
I would not give up just yet, though.

Comment 2 by foolip@chromium.org, Feb 10 2017

These are attributes on document, so very likely to be overcounted. It looks like neither property is supported by Edge 14, which should give us some hope.

In order to actually remove this, httparchive analysis would be needed. Depending on how it's used, code that will actually "break" may still be considered always in the foreground, in which case it may not be noticeable breakage to the user.

Comment 3 by tkent@chromium.org, Feb 17 2017

Components: -Blink>DOM Blink>PerformanceAPIs
Cc: foolip@chromium.org

Comment 5 by foolip@chromium.org, Mar 30 2017

Description: Show this description

Comment 6 by foolip@chromium.org, Mar 30 2017

Labels: Hotlist-GoodFirstBug
Wow, this was removed from WebKit in 2013:
https://trac.webkit.org/changeset/150695/webkit

Blink is the only engine that has this, so it should be an easy removal. An Intent to Deprecate and Remove will be needed, but I'd expect smooth sailing with the actual removal.

Comment 7 by pea...@gmail.com, Mar 31 2017

Can I upload this patch?
If so, should I update UseCounter.h and Document.idl first?
Per https://groups.google.com/a/chromium.org/d/msg/blink-dev/buY9Cbf69Ns/IxAnIkaIDAAJ the next step is to understand the usage in httparchive a bit better.

Comment 9 by panicker@google.com, Apr 13 2017

Components: -Blink>PerformanceAPIs
This appears to be mis-categorized as PerformanceAPIs

Comment 10 by phistuck@gmail.com, Apr 13 2017

Then is  issue 554834  also mis-categorized?...
Cc: rbyers@chromium.org panicker@chromium.org
Components: Blink
Labels: -Hotlist-GoodFirstBug
panicker: We don't want to loose the fact that this is a web platform bug.  Setting to just 'Blink' for now while we try to figure out what team owns page visibility APIs.

Given the initial http archive results in that thread, it sadly seems non-trivial to analyze the usage. Removing Hotlist-GoodFirstBug.  But hopefully this is just a matter of showing why a random sample of the http archive hits would not in-fact be broken by removing this API.  Given that it's removed in Edge and Safari and there is a standard replacement, it seems likely to me that removal should be ok.  But the HTTP Archive data is surprising - I agree we should try to explain it before proceeding.
Page Visibility API has been categorized to Blink>PerformanceAPIs because the Web Performance Working Group developed the standard.

Comment 13 by e...@chromium.org, May 9 2017

Components: -Blink Blink>PerformanceAPIs
Owner: igrigo...@chromium.org
Status: Assigned (was: Untriaged)
Ilya could you help provide the httparchive usage count as the next step here?
Cc: -panicker@chromium.org igrigo...@chromium.org npm@chromium.org
Owner: panicker@chromium.org
I used the instructions at https://docs.google.com/document/d/1cpjWFoXBiuFYI4zb9I7wHs7uYZ0ntbOgLwH-mgqXdEM/edit# to get the HTTP Archive hits for the PrefixedPageVisibility UseCounter:

SELECT 
  IFNULL(runs.rank, 1000000) AS rank,
  har.url AS url,
  JSON_EXTRACT(payload, '$._blinkFeatureFirstUsed.Features.PrefixedPageVisibility') AS feature
FROM [httparchive:har.2017_07_15_android_pages] AS har
LEFT JOIN [httparchive:runs.2017_07_15_pages] AS runs
ON har.url = runs.url
HAVING feature IS NOT NULL
ORDER BY rank;

And got ~181,000 rows (~40% of the archive!!!)  Here are the first 10 with their rank:
2	http://www.youtube.com/
25	http://www.linkedin.com/
30	http://www.yandex.ru/
36	http://www.ebay.com/
43	http://www.onclkds.com/
51	http://www.mail.ru/
64	http://www.wikia.com/
80	http://www.diply.com/
85	http://www.dropbox.com/
86	http://www.txxx.com/
89	http://www.coccoc.com/
91	http://www.booking.com/
92	http://www.porn555.com/
96	http://www.pixnet.net/

I assume most (if not all) of these are false-positives.  Eg. triggered by with fallback code like 'document.webkitHidden || document.hidden'.

Looking at just the first, I see the expression "a.webkitVisibilityState || a.mozVisibilityState || a.visibilityState || """ in https://pubads.g.doubleclick.net/gampad/ads

This looks like a Doubleclick ads thing, so we'd expect this one script to hit a lot of pages.

Also "var vName = d.webkitVisibilityState ? 'webkitvisibilitychange' : 'visibilitychange';" is in the main script for youtube.com.  There are a handful of other hits in the main youtube script where webkitVisibilityState is checked ahead of the others.

Given that removal was successful in Safari and Edge doesn't have it, I'd personally be OK with just trying a removal and manually verifying the above 10 sites don't start failing obviously (eg. throwing unhandled exceptions) and then just keep our eyes out for reports of issues in Beta.  I'll reply to the old intent thread with this data and proposal.

Shubhie, should we go ahead and remove these?
Components: -Blink>PerformanceAPIs Blink>PageLifecycle
Moving forward with removing sgtm.
Some of the usage was actually sites depending on it. (E.g. I think one of the youtube ones was real.) I did a code search over Google and I think I cleared out most of it, along with folks who ||'d visibilityState in the wrong order. That's the drop from 20% to 10%.

https://www.chromestatus.com/metrics/feature/timeline/popularity/196

10% is, alas, still kind of high. There are probably still some document.webkitVisibilityState || document.visibilityState ones left. There are also definitely a lot of document.hidden || document.webkitHidden. Those are particularly fun because that is a natural way to smooth over the different names of that API, but it will flag as a use for document.webkitHidden whenever the document is not hidden.
Can RAPPOR be used to find some domains that use it?
There's something called UKM now, that would probably tell us something interesting.
Splitting out webkitVisibilityState (which should be relatively robust) metrics from webkitHidden (which are a mess; I didn't "fix" the document.hidden || document.webkitHidden instances within Google either as the alternative is quite verbose, especially when lots of vendor prefixes are involved).

I could also imagine doing something really janky like: set a bit on the Document for whether document.hidden was ever accessed, and only count document.webkitHidden accesses that happen before the first document.hidden access. It has obvious false negatives, but maybe will help.

It's also worth noting that the failure mode of a site relying on a now missing document.webkitHidden is just that it thinks the page is always visible. For most sites, I expect it'd just lose some optimizations when the page is hidden. Not ideal, but not the end of the world.
Yes it seems like severity of breakage is likely to be very low.  I still think we're too worried about trying to quantify a hard-to-quantify-but-likely-low risk.  I'd support an attempted removal without further data, as long as we try to collect hard data (eg. popularity of a known impacted library) on any real regression we see and then reconsider.
My suggestion at comment 20 was more about notifying specific developers about it, regardless of removal.

#22 - why even bother keeping the fallback to vendor prefixed versions in Google code? https://caniuse.com/#search=visibility shows that it is supported unprefixed for a long time (2014). Optimizations for those old browsers are meaningless at this point. About 1% of the market and probably unsupported by Google anyway (Safari 6.1? iOS 7? Android 4.4?).
panicker@, are you still pursuing this removal? Beyond the compat analysis it should be pretty easy, so since we've done that we could add the GoodFirstBug label to see if someone wants to pick it up.

Sign in to add a comment