Closed Bug 1676466 Opened 4 years ago Closed 4 years ago

[fission] Cannot remove items from cart on lowes.com or click some image links

Categories

(Core :: Panning and Zooming, defect, P2)

defect

Tracking

()

RESOLVED WORKSFORME
Fission Milestone M6c

People

(Reporter: agi, Assigned: mikokm)

References

Details

(Whiteboard: [2/3] patches in bug 1534549)

Attachments

(2 files)

Just noticed that, with Fission enabled I cannot remove items from cart at https://www.lowes.com/cart

STR

navigate to https://www.lowes.com
add items to cart
navigate to https://www.lowes.com/cart
the whole left panel is unresponsive, cannot select/remove click on links

Summary: Cannot remove items to cart on lowes.com → [fission] Cannot remove items from cart on lowes.com
Fission Milestone: --- → ?

@ Emilio, kmag says lowes.com has a cross-origin iframe that overlays the entire page. Do you know why this might break click event handling with Fission? Is this a Layout bug?

Nika can't reproduce on the lowes.ca site in Canada, so you might need to use a VPN or proxy to reproduce using the lowes.com US site.

Simpler STR:

  1. Load https://www.lowes.com/l/gift-ideas.html?int_cmp=Home:A1:NoDivision:Other:FW35_20_Gifting
  2. Try to click the giant "gift zone" image or "Shop All" button.
  3. They are unclickable in a Fission window but clickable in a non-Fission window.

kmag thinks this bug is serious enough to block increasing our Nightly experiment population.

Severity: -- → S2
Fission Milestone: ? → M6c
Flags: needinfo?(emilio)
Priority: -- → P2
Summary: [fission] Cannot remove items from cart on lowes.com → [fission] Cannot remove items from cart on lowes.com or click some image links
Attached file Reduced test-case (deleted) —
Flags: needinfo?(emilio)
Attachment #9187836 - Attachment description: Reduced test-c → Reduced test-case

So what's going on is that we're not correctly respecting z-index on the container (we do that properly for painting, though).

I'm not quite familiar with how hit testing is hooked up for Fission frames... Maybe Matt knows? I'm happy to poke at this with some pointers, it might be either an APZ bug or a DOM:Events bug I guess. It seems some code is not properly accounting for iframes not being in the foreground.

Flags: needinfo?(matt.woodrow)

APZ Fission hit-testing bug 1519412 might be related. Kats added some detailed notes in that bug.

Matt confirmed on Matrix that it's probably the same issue as Bug 1541589.

I'm in Canada too and able to reproduce not being able to remove items from cart on https://www.lowes.com/ .
Also able to reproduce inability to click on links with these steps.
Click on:
Shop -> Holiday Decorations -> Shop All Holiday Decorations
This page has picture links for different categories such as "Christmas Decorations", "Fall Decorations", etc. , none of which are click-able with Fission.

I tried in non-Fission window and it works fine there.

Flags: needinfo?(matt.woodrow)

Botond, this is a pretty serious issue with Fission. Do you know who could work on this?

Component: DOM: Navigation → Panning and Zooming
Flags: needinfo?(botond)

This looks like a display list building bug. Specifically, I think one of the optimizations to reduce the number of CompositorHitTestInfo items is the root cause.

In the attached display list, which is that for the root content process of emilio's reduced test case, we see the Remote item on line 26 which is for the OOPIF. And then later (lines 29, 32, 34) we see the background/border/content of the button. However there are no hit-test info items specifically for the button; the topmost hit-test info items are for the container element (on line 22, with zindex=-1) and for the OOPIF (on line 24).

I would expect to see the hit-test info items for the button alongside the regular display items for the button, and which would the end up on "top" of the OOPIF, but I believe they got optimized away and considered part of root canvas hit-test info (line 17).

Miko, I believe you are the most familiar with the hit-test info optimizations, does what I'm saying make sense and/or can you point us to the relevant code?

Flags: needinfo?(mikokm)

(In reply to Kartikaya Gupta (email:kats@mozilla.staktrace.com) from comment #8)

Created attachment 9188372 [details]
Display list dump for root content process for attached reduced testcase

This looks like a display list building bug. Specifically, I think one of the optimizations to reduce the number of CompositorHitTestInfo items is the root cause.
Miko, I believe you are the most familiar with the hit-test info optimizations, does what I'm saying make sense and/or can you point us to the relevant code?

I think you are correct. This does seem like it could be caused by the issue described in bug 1534549. I verified this by commenting out the early return that avoids creating multiple items for the same hit test area1, and this fixes the testcase (as well as the original site).

Depends on: 1534549
Flags: needinfo?(mikokm)

I am looking into this.

Assignee: nobody → mikokm
Status: NEW → ASSIGNED

Thanks Miko!

Flags: needinfo?(botond)
Blocks: 1679320
Blocks: 1519412
Whiteboard: [2/3] patches in bug 1534549

Now that the patches for bug 1534549 have landed, the reduced testcase (and presumably the original site) works. Marking this as fixed.

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

I've also tested with Fission in Nightly on lowes.com, could not reproduce.

Status: RESOLVED → VERIFIED

I did expect this bug to add a test for the iframe cae. Isn't it worth doing?

I've also tested with Fission in Nightly 87.0a1 (2021-02-17) (64-bit) on lowes.com, could not reproduce. I was able to remove items or click on gift cards image or shop all. Considering it does not seem to reproduce, I will close this issue as worksforme.

Status: VERIFIED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: FIXED → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: