Bug 143609

Polyfill: https://github.com/mathiasbynens/document.scrollingElement

I saw your email on www-style. Thanks for filing a bug report here. I'd rather not have scrollingElement and just fix the damn bug :)

Even when the bug is fixed in WebKit and Blink, scrollingElement still makes sense to use for libraries that want to support being used in quirks mode.

(In reply to comment #3) > Even when the bug is fixed in WebKit and Blink, scrollingElement still makes > sense to use for libraries that want to support being used in quirks mode. Tell me more. How would you use scrollingElement in a way that can't be covered by existing APIs?

I'd rather just fix the damn bug too. The problem is that we're finding lots of sites that have been forced to guess at the scrollingElement using a UA check (here's an example: https://github.com/google/closure-library/blob/32365aba43acb36c5d693256ef5d4dbe3bddddfe/closure/goog/dom/dom.js#L632, and Facebook had something similar). I need some good/simple/reliable guidance for those sites to migrate to the spec-compatible behavior. After a number of failed attempts, I don't see any path for switching blink to the correct behavior without first adding such an API. Thoughts?

(In reply to comment #5) > I'd rather just fix the damn bug too. The problem is that we're finding > lots of sites that have been forced to guess at the scrollingElement using a > UA check (here's an example: > https://github.com/google/closure-library/blob/ > 32365aba43acb36c5d693256ef5d4dbe3bddddfe/closure/goog/dom/dom.js#L632, and > Facebook had something similar). I need some good/simple/reliable guidance > for those sites to migrate to the spec-compatible behavior. After a number > of failed attempts, I don't see any path for switching blink to the correct > behavior without first adding such an API. Thoughts? The problem here is the UA check for detecting behavior. If it is just a matter of feature detection, one could change scrollTop/Left on body, observe which viewport has been modified, then restore the scroll position. What I do not like about "scrollingElement" is that it looks like a temporary hack. It goes against keeping the platform simple.

(In reply to comment #6) > (In reply to comment #5) > > I'd rather just fix the damn bug too. The problem is that we're finding > > lots of sites that have been forced to guess at the scrollingElement using a > > UA check (here's an example: > > https://github.com/google/closure-library/blob/ > > 32365aba43acb36c5d693256ef5d4dbe3bddddfe/closure/goog/dom/dom.js#L632, and > > Facebook had something similar). I need some good/simple/reliable guidance > > for those sites to migrate to the spec-compatible behavior. After a number > > of failed attempts, I don't see any path for switching blink to the correct > > behavior without first adding such an API. Thoughts? > > The problem here is the UA check for detecting behavior. > > If it is just a matter of feature detection, one could change scrollTop/Left > on body, observe which viewport has been modified, then restore the scroll > position. That might result in a noticeable visible artifact (especially for code that's invoking such 'find me the scrolling element' logic regularly). If you plan to cache the result then you need to also force the page to be scrollable somehow during the test. The polyfill referenced above addresses all these issues by creating a temporary iframe to run the test, and as a result is starting to get a little complex (hard to argue that it's a good drop in replacement for the simple UA check folks are doing). But I can see the argument saying that's a better path forward. > What I do not like about "scrollingElement" is that it looks like a > temporary hack. It goes against keeping the platform simple. I agree it would be nicer if we could correct our mistakes only through simplifying the platform API surface area. But breaking changes (even bug fixes) are really hard to do. Personally I'm willing to add simple, well-defined APIs when it simplifies the platform mental model even while adding more surface area. FWIW I expect the presence of this API to simplify the spec text around all this dramatically, as it has simplified our implementation in blink - crrev.com/1075393002 (both of course could have been done by introducing the concept without exposing it). Anyway, pragmatically it feels to me like our choices are to add an API like this to answer the exact question developers are attempting (and failing) to guess or to stick with the status quo where browsers that pretend to be WebKit in their UA (now including MS EdgeHTML) have to have a different implementation than those that don't. I'm happy to be proven wrong if someone else wants to pick up the torch of trying to get this platform wart fixed a different way - it looks like bug 106133 hasn't had any love in awhile :-( Also there is the minor argument Simon alluded to which says it's simpler and more reliably for library developers to use scrollingElement than to figure out they need to determine whether or not they're in quirks mode. But I agree that's a pretty minor simplicity benefit, not enough to justify this API on it's own.

What is Mozilla's take on this?

(In reply to comment #8) > What is Mozilla's take on this? Looks like they plan to implement it: https://bugzilla.mozilla.org/show_bug.cgi?id=1153322

(In reply to comment #9) > (In reply to comment #8) > > What is Mozilla's take on this? > > Looks like they plan to implement it: > https://bugzilla.mozilla.org/show_bug.cgi?id=1153322 I am never a fan of adding something that can easily be implemented in JavaScript. But if both Mozilla and Chrome want the added complexity...let's do it.

<rdar://problem/20845213>

Created attachment 252661 [details] Patch

Comment on attachment 252661 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=252661&action=review > Source/WebCore/ChangeLog:3 > + Consider implementing Document.scrollingElement No longer considering!

Committed r183967: <http://trac.webkit.org/changeset/183967>