2055039 - test_queryCaretRect.html caret-position assertions fail when Nova is enabled

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Stephen Thompson [:sthompson]]

dom/tests/mochitest/chrome/test_queryCaretRect.html fails on Nova Tier-3 test runs, across all platforms, with repeated caret-position assertion failures.

Dashboard: https://tests.firefox.dev/test.html?test=dom/tests/mochitest/chrome/test_queryCaretRect.html

Failures observed:

• [win, linux, mac] "rect.top < 390" fails repeatedly across many caret-position sub-tests (queryCaretRectWin.html / queryCaretRectUnix.html)

See e.g. https://treeherder.mozilla.org/jobs?repo=mozilla-central&group_state=expanded&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&tier=1%2C2%2C3&searchStr=nova&revision=88b0f8e6a5925058755d2671bdac5a76d84bd224&selectedTaskRun=Jh3XFO6NQ96lOlQKiIK3gQ.0

Generally, the selection rect is further right or further down by a few more pixels than expected. The browser chrome takes up additional space under Nova, but that shouldn't affect measurements that occur only within the client area.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Looks like the test assumes the editor's rect because the expectation is fixed values:
https://searchfox.org/firefox-main/rev/9822ee9dfe9be4851b0466ebf21646d66a5b7960/dom/tests/mochitest/chrome/queryCaretRectWin.html#89-92,112-113

but Nova changed that. I think the expected value should be changed to refer the editor's rect.

The browser chrome takes up additional space under Nova, but that shouldn't affect measurements that occur only within the client area.

The result is relative to the top level widget. Therefore, if the toolbar height is changed by applying Nova, this test may fail.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The test is anyway not aware of HiDPI...

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 2055039 - Make `test_queryCaretRect.html` check with dynamic data r=m_kato!

The test checks the query result point relative to the root widget
with the fixed value. Therefore, if the widget is not at the top-left
corner of the top level widget, it fails to compare the expected values.
Additionally, the test is not HiDPI environment aware. Therefore, this
makes the test check window.devicePixelRatio.

Finally, we don't treat CRLF on Windows separately. Therefore, we don't
need to have the CRLF version of the test anymore.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

How can I test the result with enabling Nova?

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Ah, okay, --setpref browser.nova.enabled=true allows that. However, indeed, even the patched test fails with bigger left and top values.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

In my environment (200%), QUERY_EDITOR_RECT returns:

editor rect={x=8, y=7, width=1600, height=1200}

but with enabling Nova, the result is changed to:

editor rect={x=18, y=28, width=1600, height=1200}

So, the <textarea> was moved 5 CSS pixels right and 10.5 CSS pixels bottom.

However, window.mozInnerScreenX - window.top.mozInnerScreenX and window.mozInnerScreenY - window.top.mozInnerScreenY are 0. So, I'm not sure how to detect the actual offset of the window in the top level widget...

Emilio, do you know how to get it?

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Okay, using the offset of QUERY_EDITOR_RECT works, but I'm still interested in how to get the window offset in the top level widget in the JS world. So, not canceling the ni.

Comment 8

[Emilio Cobos Álvarez [:emilio]]

This is because window == window.top right? You're running in a <browser> that isn't a direct child of browser.xhtml... Does mozInnerScreenX - screenX work here? If not you'd need to use a privileged API (not .top) to get to the toplevel window. And of course it'd only work in the chrome mochitest in this case. I.e., this should work:

window.mozInnerScreenX - window.browsingContext.topChromeWindow.mozInnerScreenX

Doesn't it?

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thanks, it gets different result in Nova. So, I guess it works. But simply using the value breaks the patch. I'll update the patch with it...

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/1fcbd16b26c0
https://hg.mozilla.org/integration/autoland/rev/55a71de2cd58
Make `test_queryCaretRect.html` check with dynamic data r=m_kato

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/55a71de2cd58