Closed Bug 1524854 Opened 6 years ago Closed 5 years ago

Changing sort order within Sephora search results does not work

Categories

(Web Compatibility :: Site Reports, defect, P3)

Desktop
macOS
defect

Tracking

(firefox67 affected)

RESOLVED INVALID
Tracking Status
firefox67 --- affected

People

(Reporter: shiggins, Unassigned)

References

()

Details

(Keywords: webcompat:needs-diagnosis, Whiteboard: [webcompat:sightline])

Attachments

(1 file)

Drop down menu on top commerce site launches but click action does not work when trying to sort results...

https://www.sephora.com/search?keyword=make%20up%20primer.

Checked on Chrome and it works fine.

I can reproduce this on Nightly in a clean profile with no tracking protection.

Component: General → Desktop
OS: iOS 10 → All
Product: Firefox → Tech Evangelism
Summary: Clicking on drop down menu within a site search results does not work → Changing sort order within Sephora search results does not work

The filter label is not even changing.
no error message in the console.

The markup for this section.

<div
class="css-wgdrb6 "
data-comp="ProductSortFS ActionMenu Dropdown Box"
>
<div
    id="cat_sort_menu_trigger"
    aria-controls="cat_sort_menu"
    aria-haspopup="true"
    aria-describedby="catalogSortDescription"
    class="css-1gw67j0 "
    role="button"
    tabindex="0"
    data-comp="DropdownTrigger Link Box"
>
    Sort by: <b>Relevancy</b
    ><svg
    aria-hidden="true"
    viewBox="0 0 95 57"
    class="css-2tbuo7 "
    data-comp="Chevron Box"
    >
    <path
        d="M47.5 57L95 9.5 85.5 0l-38 38-38-38L0 9.5 47.5 57z"
    ></path></svg
    ><span id="catalogSortDescription" style="display:none"
    >Choosing sorting option will automatically update the
    products that are displayed to match the selected
    sorting option</span
    >
</div>
<div
    style="display:none"
    class="css-8107hs DropdownMenu"
    role="none"
    data-comp="DropdownMenu Box"
>
    <div
    id="cat_sort_menu"
    aria-labelledby="cat_sort_menu_trigger"
    class="css-wiigih "
    role="menu"
    data-comp="Box"
    >
    <button
        aria-current="true"
        class="css-1wlcnu "
        type="button"
        role="menuitem"
        data-comp="Link Box"
    >
        Relevancy</button
    ><button
        class="css-1hkefft "
        type="button"
        role="menuitem"
        data-comp="Link Box"
    >
        Bestselling</button
    ><button
        class="css-1hkefft "
        type="button"
        role="menuitem"
        data-comp="Link Box"
    >
        Top Rated</button
    ><button
        class="css-1hkefft "
        type="button"
        role="menuitem"
        data-comp="Link Box"
    >
        Exclusive</button
    ><button
        class="css-1hkefft "
        type="button"
        role="menuitem"
        data-comp="Link Box"
    >
        New</button
    ><button
        class="css-1hkefft "
        type="button"
        role="menuitem"
        data-comp="Link Box"
    >
        Price High to Low</button
    ><button
        class="css-1hkefft "
        type="button"
        role="menuitem"
        data-comp="Link Box"
    >
        Price Low to High</button
    ><button
        class="css-1hkefft "
        type="button"
        role="menuitem"
        data-comp="Link Box"
    >
        Brand Name
    </button>
    </div>
</div>
</div>

A series of buttons to emulate a select… o_O

Dropdown.prototype.render = function () {
    const {
        id,
        isHover,
        isStatic,
        onTrigger,
        syncState,
        hasSubmenu,
        delayedHover,
        ...props
    } = this.props;

    /* onTouchStart is used exclusively for tablets in dropdown. The reason is that in iOS safari
    element.focus() can only be set programatically when inside an actual touch event. This is
    needed so that the onBlur event can close it when clicked outside. */

    return (
        <Box
            {...props}
            baseCss={{
                position: isStatic ? 'static' : 'relative'
            }}
            onTouchStart={(!this.isHover && Sephora.isTouch) ? this.focusElement : null}
            onBlur={!this.isHover ? e => {
                if (ReactDOM !== undefined) {
                    const element = ReactDOM.findDOMNode(this);
                    const menu = element.querySelectorAll('.DropdownMenu')[0];

                    /* Given to differences in how browsers interpret event dispatchers and
                    targets, we must check what current activeElement per focusout is, or
                    event.explicitOriginalTarget in the case of FireFox. We close upon blur
                    if said element is not within the dropdown's menu.
                    */

                    /* TODO 17.7: Find a more sensible way to handle this differences and address
                    why trigger is sometimes called twice in the case of Chrome, and handle how the
                    handler addressed bubbling/propagation. */

                    if (menu && !menu.contains(e.relatedTarget ||
                        document.activeElement ||
                        e.explicitOriginalTarget)) {
                        this.triggerDropdown(e, false);
                    }
                }
            } : null}>
            {
                React.Children.map(this.props.children,
                    (child, index) => child && React.cloneElement(child,
                        {
                            key: index.toString(),
                            id: id,
                            triggerDropdown: this.triggerDropdown,
                            open: this.state.isActive || syncState,
                            isHover: this.isHover
                        }
                    ))
            }
        </Box>
    );
};

hmmm I wonder if it was working before we fixed the button/target issue: Bug 1089326
What do you think Thomas?

Flags: needinfo?(twisniewski)
Whiteboard: [webcompat] [needsdiagnosis]

Oddly, I'm only able to reproduce this on OSX Macbook. My fast Linux desktop works fine, and my slow Quantum reference laptop with Windows 10 also works fine.

Also, according to mozregression, it wasn't working even as far back as version 52, so I think there's something stranger going on here that warrants a deeper investigation.

Flags: needinfo?(twisniewski)

First suspecting a weird CSS glitch, I removed all styles (except for one needed to be able to click the sort option: .css-2tbuo7 { width:1em }). No luck.

My next hypothesis was a faulty blur handler, so I told Tinker Tester to ignore blur events. Sure enough, it caused the popup to not vanish anymore.

It seems that the blur event is triggered as the user mousedowns on the sort popup (#cat_sort_menu_trigger). The events that are fired are a mousedown, blur, and click in both Firefox and Chrome, to the same event handlers: React event handlers. They ultimately do an interactiveUpdates call that does a commitWork which sets the display of the menu (.css-8107gs.DropdownMenu) to "none" during the blur event.

This lead me to do a quick search for known issues like this against React, which yielded a result: https://github.com/facebook/react/issues/12993

Ah, an OSX-only interop issue, apparently to do with button-clicks.

Sure enough, the items inside of their menu are actually buttons; the outerHTML of the element being hidden looks like this:

<div style="display: none;" class="css-8107hs DropdownMenu" role="none" data-comp="DropdownMenu Box">
<div id="cat_sort_menu" aria-labelledby="cat_sort_menu_trigger" class="css-wiigih " role="menu" data-comp="Box">
<button aria-current="true" class="css-1wlcnu " type="button" role="menuitem" data-comp="Link Box">Relevancy<gbutton>
<button class="css-1hkefft " type="button" role="menuitem" data-comp="Link Box">Bestselling</button>
<button class="css-1hkefft " type="button" role="menuitem" data-comp="Link Box">Top Rated</button>
<button class="css-1hkefft " type="button" role="menuitem" data-comp="Link Box">Exclusive</button>
<button class="css-1hkefft " type="button" role="menuitem" data-comp="Link Box">New</button>
<button class="css-1hkefft " type="button" role="menuitem" data-comp="Link Box">Price High to Low</button>
<button class="css-1hkefft " type="button" role="menuitem" data-comp="Link Box">Price Low to High</button>
<button class="css-1hkefft " type="button" role="menuitem" data-comp="Link Box">Brand Name</button>
</div>
</div>

And true to the analysis on that React bug, Safari is also experiencing this problem.

Sadly it seems that nobody cared to report this to us or Webkit, and the React bug was simply closed with a "lol interop" shrug.

I'm not at all sure where to go from here, though. Either the site has to not use buttons, or we and Webkit have to figure out how to resolve this interop issue (which seems wise given that this is related to React). What do you think, Mike?

Flags: needinfo?(miket)
OS: All → macOS

Tom, can we get a reduced test case? Seems useful to at least get a bug filed against whatever is the relevant spec, and consider changing our behavior to match what Chrome does.

Flags: needinfo?(miket) → needinfo?(twisniewski)
Attached file test.html

Sure, I've copied the test used on that React issue here for safekeeping.

Flags: needinfo?(twisniewski)
Product: Tech Evangelism → Web Compatibility

Migrating Webcompat whiteboard priorities to project flags. See bug 1547409.

Webcompat Priority: --- → ?

See bug 1547409. Migrating whiteboard priority tags to program flags.

See bug 1547409. Moving webcompat whiteboard tags to keywords.

Anne, are you aware of any bugs related to MouseEvent not bubbling focus/blur event (only on MacOS?).

Flags: needinfo?(annevk)
Priority: -- → P3
Whiteboard: [webcompat] [needsdiagnosis]

Olli might know more, but in general macOS has different focus handling from other platforms and this can cause these kind of issues.

Flags: needinfo?(annevk) → needinfo?(bugs)

We should have similar focus model to Safari, I believe. I don't have MacOS atm to test.
And the focus model is that because of platform conventions.

Flags: needinfo?(bugs)

This site has since been fixed, and since we're matching Safari let's close as invalid.

Status: NEW → RESOLVED
Closed: 5 years ago
Webcompat Priority: ? → ---
Resolution: --- → INVALID
Whiteboard: [webcompat:sightline]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: