Skip to content

fix(platform): detect and ignore scrollBehavior polyfills #20155

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 19, 2020
Merged

fix(platform): detect and ignore scrollBehavior polyfills #20155

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion src/cdk/platform/features/scrolling.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,38 @@ export const enum RtlScrollAxisType {
/** Cached result of the way the browser handles the horizontal scroll axis in RTL mode. */
let rtlScrollAxisType: RtlScrollAxisType|undefined;

/** Cached result of the check that indicates whether the browser supports scroll behaviors. */
let scrollBehaviorSupported: boolean|undefined;

/** Check whether the browser supports scroll behaviors. */
export function supportsScrollBehavior(): boolean {
return !!(typeof document == 'object' && 'scrollBehavior' in document.documentElement!.style);
if (scrollBehaviorSupported == null) {
// If we're not in the browser, it can't be supported.
if (typeof document !== 'object' || !document) {
scrollBehaviorSupported = false;
}

// If the element can have a `scrollBehavior` style, we can be sure that it's supported.
if ('scrollBehavior' in document.documentElement!.style) {
scrollBehaviorSupported = true;
} else {
// At this point we have 3 possibilities: `scrollTo` isn't supported at all, it's
// supported but it doesn't handle scroll behavior, or it has been polyfilled.
const scrollToFunction: Function|undefined = Element.prototype.scrollTo;

if (scrollToFunction) {
// We can detect if the function has been polyfilled by calling `toString` on it. Native
// functions are obfuscated using `[native code]`, whereas if it was overwritten we'd get
// the actual function source. Via https://davidwalsh.name/detect-native-function. Consider
// polyfilled functions as supporting scroll behavior.
Copy link
Member

Choose a reason for hiding this comment

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

I don't quite understand- if we treat a polyfill as fully supported, why check for a native implementation?

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that pretty much all browsers support these two signatures: scrollTo(options: {top: number, left: number, behavior: string}) or scrollTo(top: number, left: number), whereas Safari only supports the latter. If we were to pass in an object to Safari nothing would happen so we have to detect if it's polyfilled. A bit above we detect whether the behavior parameter is supported which is a good indicator that the object is supported too.

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this check return true for Safari, then, when the behavior config isn't supported?

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 messed it up here, because I didn't negate the check. It should be correct now.

Copy link
Member

Choose a reason for hiding this comment

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

Yay code review

Copy link

Choose a reason for hiding this comment

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

I used to check for focusOptions preventScroll support with this technique:

    let supported = false;

    document.createElement('div').focus({
        get preventScroll() {
            supported = true;

            return false;
        },
    });

Looks like it could work here too, to check if browser tries to retrieve value of behavior key in a passed object.

scrollBehaviorSupported = !/\{\s*\[native code\]\s*\}/.test(scrollToFunction.toString());
} else {
scrollBehaviorSupported = false;
}
}
}

return scrollBehaviorSupported;
}

/**
Expand Down