1937115 - Clicking on element doesn't work if element is in iframe outside of viewport

Status: NEW

(Core :: Layout: Scrolling and Overflow, defect, P3)

Description

[Nikolay]

Attachment: HTML_Page.png

Steps to reproduce:

Using WebDriver to interact with the page it stopped to click on specific element.
I have simple page with iframe and a link inside. I want to click on this link.

The issue is not reproducible in Firefox v133.0.
The issue appeared firstly in Firefox v134.0b1 and only on Linux (Ubuntu 22), it is not reproducible on Windows and MacOS.

Actual results:

Nothing happened on Linux. In logs I see:

1734100134899 Marionette WARN TimedPromise timed out after 500 ms: stacktrace:
TimedPromise/<@chrome://remote/content/marionette/sync.sys.mjs:206:24
TimedPromise@chrome://remote/content/marionette/sync.sys.mjs:190:10
interaction.flushEventLoop@chrome://remote/content/marionette/interaction.sys.mjs:467:10
webdriverClickElement@chrome://remote/content/marionette/interaction.sys.mjs:220:33

The mozregression tool found the following changeset:

2024-12-13T17:29:05.697000: DEBUG : Found commit message:
Bug 1931466: Ensure inline elements in an empty linebox as having zero BSize and no baseline. r=layout-reviewers,dholbert,jfkthame

Also add meta viewport tag to the setup of /FileAPI/url/url-in-tags.window.js
WPT - This test passes now, but without this change, the test's viewport gets
automatically zoomed out when run on Android; and that zoom factor can lead to
off-by-one test failures from pixel-rounding discrepancies.

Differential Revision: https://phabricator.services.mozilla.com/D229191

Expected results:

The link should be clicked.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Panning and Zooming' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

As it looks like the click doesn't reach the element itself and as such no navigation to the target page happens.

I've given Nikolay the instructions in how to run mozregression to figure out the exact regression. Not sure if that particular patch is really the regressor given that it doesn't seem to be Linux only.

David, do you have an idea?

Comment 3

[David Shin[:dshin] (PTO Til 24th)]

Hm - yeah, it's strange that it's Linux only (And not related to Android).
Are you able to attach the testcase? Need to dig in to figure out what's going on...

Comment 4

[Nikolay]

In C#:

// use "134.0b1" to reproduce the issue, in "133.0" it works
var options = new FirefoxOptions { BrowserVersion = "134.0b1", LogLevel = FirefoxDriverLogLevel.Trace };
using var driver = new FirefoxDriver(options);

driver.Url = "http://desktop-sm1doe9.local:54270/common/click_too_big_in_frame.html"; // the page DOM I already attached

IWebElement frame = driver.FindElement(By.Id("iframe1"));

driver.SwitchTo().Frame(frame);

IWebElement element = driver.FindElement(By.Id("click"));

element.Click();

Thread.Sleep(20_000); // just to see what is happening, we should be navigated to another page

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: HTML testcase.zip

I've extracted these files from the Selenium repository. A Marionette test would look like the following:

    def test(self):
        self.marionette.navigate(
            "file:///path/to/frameset.html")
        self.marionette.switch_to_frame(0)
        elem = self.marionette.find_element(By.ID, "click")
        elem.click()

        Wait(self.marionette, timeout=10).until(
            lambda _: self.marionette.title == "clicks")

Comment 6

[David Shin[:dshin] (PTO Til 24th)]

Mm, right. #click's getClientRects 3 boxes, before and after Bug 1931466 .
Second box is identical for before/after - it's the box for the inner <div>.
First/last box returned used to be of zero width but of some height, after bug 1931466, it's zero sized.

Marionette positions in the middle of the first rect before clicking, and it seems like that causes that click event to be eaten.

Comment 7

[David Shin[:dshin] (PTO Til 24th)]

(Note that there's also something very specific about the testcase - modifications from the exact case tends to hide the issue)

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to David Shin[:dshin] from comment #6)

First/last box returned used to be of zero width but of some height, after bug 1931466, it's zero sized.

Interesting and thanks for taking a look that quickly David! So I'm going to mark this bug as a regression of bug 1931466 then.

Marionette positions in the middle of the first rect before clicking, and it seems like that causes that click event to be eaten.

Despite the above fact if there is something we should improve for Marionette please let us know but at the moment it looks like a platform issue that we were able to hit with that particular usage in Marionette.

Comment 9

[Jens Stutte [:jstutte]]

(In reply to David Shin[:dshin] from comment #6)

Mm, right. #click's getClientRects 3 boxes, before and after Bug 1931466 .
Second box is identical for before/after - it's the box for the inner <div>.
First/last box returned used to be of zero width but of some height, after bug 1931466, it's zero sized.

If I read this correctly we were changing an already invisible element (with some height) to an even more invisible element.

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #8)

(In reply to David Shin[:dshin] from comment #6)

Marionette positions in the middle of the first rect before clicking, and it seems like that causes that click event to be eaten.

Despite the above fact if there is something we should improve for Marionette please let us know but at the moment it looks like a platform issue that we were able to hit with that particular usage in Marionette.

Does this make the problem kind of less relevant for normal users ? Am I understanding right that marionette simulates a click as if the user would have clicked with the mouse at a certain position where there is, uhm, nothing (visible) ? Or would Javascript running inside the browser and trying to do a click() on some such element be affected by this, too?

(I am just trying to understand the severity to be able to set the tracking flags for 134 well)

Comment 10

[David Shin[:dshin] (PTO Til 24th)]

Ugh. Sorry, I was misled by trying to minimize the case in a hurry. Dug into Bug 1937115 comment 7 a bit more, and realized that adding <!DOCTYPE html> fixes this. So the issue is not with clicking on zero-sized element. Digging a bit more, removing <h1> also fixes the issue.

In Quirks Mode, <h1>'s margin-top is set to zero for being the first element in <body>, from the UA sheet default of 0.67em. To that end, adding style="margin-top: 0.67em" also fixes the issue.

Investigation is ongoing to pin down the issue.

Comment 11

[David Shin[:dshin] (PTO Til 24th)]

Ok. Rather nasty issue that Bug 1931466 uncovered.

Given frames like this:

Viewport(-1)@765349376020 [view=765349db1dd0] (x=0, y=0, w=76800, h=57300) [cs=765349dc3108:-moz-viewport] <
  ScrollContainer(html)(-1)@7653493761b8 parent=765349376020 (x=0, y=0, w=76800, h=57300) [content=765347b03040] [cs=765349dc0808:-moz-viewport-scroll] <
    ScrollbarFrame(scrollbar)(-1)@765349376460 parent=7653493761b8 next=7653493766d8 (x=0, y=56580, w=76800, h=720) [content=765347b031f0] [cs=76534712b108] modified <
      SliderFrame(slider)(-1)@765349376560 parent=765349376460 (x=0, y=0, w=76800, h=720) [content=765347b033a0] [cs=7653493a8f08] <
        Frame(thumb)(0)@765349376660 parent=765349376560 (x=0, y=0, w=8040, h=720) [content=765347b060e0] [cs=7653493a7608]
      >
    >
    ScrollbarFrame(scrollbar)(-1)@7653493766d8 parent=7653493761b8 next=765349376950 (x=76080, y=0, w=720, h=57300) [content=765347b03280] [cs=76534712c308] modified <
      SliderFrame(slider)(-1)@7653493767d8 parent=7653493766d8 (x=0, y=0, w=720, h=57300) [content=765347b03430] [cs=76534712bf08] <
        Frame(thumb)(0)@7653493768d8 parent=7653493767d8 (x=0, y=0, w=720, h=5160) [content=765347b06190] [cs=76534712c608]
      >
    >
    Frame(scrollcorner)(-1)@765349376950 parent=7653493761b8 next=7653493760d8 (x=76800, y=57300, w=0, h=0) [content=765347b03310] [cs=765349dc3b08]
    Canvas(html)(-1)@7653493760d8 parent=7653493761b8 (x=0, y=0, w=76800, h=57300) ink-overflow=(x=0, y=0, w=600540, h=604646) scr-overflow=(x=0, y=0, w=600540, h=604646) [content=765347b03040] [cs=765349dc3508:-moz-scrolled-canvas] <
      Block(html)(-1)@7653493769c8 parent=7653493760d8 (x=0, y=0, w=76800, h=604646) ink-overflow=(x=0, y=0, w=600540, h=604646) scr-overflow=(x=0, y=0, w=600540, h=604646) [content=765347b03040] [cs=765349dc2408] <
        line@765349376b58 count=1 state=block,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:nonebm=480 (x=480, y=480, w=75840, h=603686) ink-overflow=(x=480, y=480, w=600060, h=603686) scr-overflow=(x=480, y=480, w=600060, h=603686) <
          Block(body)(2)@765349376a90 parent=7653493769c8 (x=480, y=480, w=75840, h=603686) ink-overflow=(x=0, y=0, w=600060, h=603686) scr-overflow=(x=0, y=0, w=600060, h=603686) [content=765347b03160] [cs=765349dc3208] <
            line@765349376e68 count=1 state=block,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:nonebm=1286 (x=0, y=0, w=75840, h=2340) <
              Block(h1 id=iframe_page_heading)(1)@765349376ba8 parent=765349376a90 next=765349376d60 (x=0, y=0, w=75840, h=2340) [content=765347b03550] [cs=765349dc3708] <
                line@765349376d10 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:none(x=0, y=0, w=20627, h=2340) <
                  Text(0)"This is the heading"@765349376c70 parent=765349376ba8 (x=0, y=0, w=20627, h=2340) [content=765347b04300] [cs=765349dc3a08:-moz-text] [run=765349d9a940][0,19,T]
                >
              >
            >
            line@765349376eb8 count=1 state=inline,clean,prevmarginclean,not-impacted,not-wrapped,no-break,clear-before:none,clear-after:none(x=0, y=3626, w=600060, h=600060) <
              FrameOuter(iframe id=iframe1 src=test_bug1937115_inner.html)(3)@765349376d60 [view=765349db1120] parent=765349376a90 (x=0, y=3626, w=600060, h=600060) [content=765347b07040] [cs=765349dc3808]
            >
          >
        >
      >
    >
  >
>

We get the inner frame's first line rect, which is zero sized, so we try to click at the upper left corner of the <iframe>.
Given the <body> margins, at viewport level, the location of the click point is (480, 4106) app units (8, 68.43[..] pixels).
Click event is rounded here with call site here, to (8, 68) pixels, which is (480, 4080) app units.
We build a DisplayList starting from viewport, drilling down to relevant lines containing the <iframe>, by creating a hit test for (1x1) app unit sized rectangle with top left at (480, 4080) app units. However, the line containing <iframe> is located at (480, 4106) app units, which does not pass the hit test, so we end up making that line not part of the hit test.

Previously, because the line has (x=0, y=-900, w=0, h=1140) (in app units), the center point (Of visible part) is 120 app units/2 pixels, nudging down from the upper left corner of the <iframe>, and overcoming the rounding error.

Quirks Mode & h1 margin stuff basically a red herring - it merely puts rounding in a way that just happens to work out.

Yikes, this is... really unfortunate. I think the rounding should be avoided in the first place, but it assigns to mRefPoint in device pixel integer, so that'd be a bigger change. I guess it's possible to increase the size of hit test rectangle, but there could be unforeseen correctness/performance issues there..

Comment 12

[Jens Stutte [:jstutte]]

Rather nasty issue that Bug 1931466 uncovered.

What does this mean for the severity here?

Comment 13

[David Shin[:dshin] (PTO Til 24th)]

I'd say S3, since it would really become an issue in a single-pixel band with few-pixels thin clickable elements, which hopefully aren't that common in the wild (You could argue that block-in-inline as used in the testcase causes this as well, but users in general wouldn't end up clicking on the zero-height bounding box like the testcase does).
Though, it clearly does affect tests, so I'll look at fixes sooner than later.