1905330 - Perma layout/reftests/writing-mode/1083848-1-inline-border.html == layout/reftests/writing-mode/1083848-1-inline-border-ref.html | image comparison, max difference: 255, number of differin when Gecko 129 switches from Early Beta to Late Beta on 2024-07-19

Status: VERIFIED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

Central-as-beta simulation
How to run these simulations
Failure log:

[task 2024-06-28T11:05:42.353Z] 11:05:42     INFO - REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/writing-mode/1083848-1-inline-border.html == layout/reftests/writing-mode/1083848-1-inline-border-ref.html | image comparison, max difference: 255, number of differing pixels: 1965
(...)
[task 2024-06-28T11:05:42.358Z] 11:05:42     INFO - REFTEST INFO | Saved log: START file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/writing-mode/1083848-1-inline-border.html
[task 2024-06-28T11:05:42.359Z] 11:05:42     INFO - REFTEST INFO | Saved log: [CONTENT] OnDocumentLoad triggering AfterOnLoadScripts
[task 2024-06-28T11:05:42.362Z] 11:05:42     INFO - REFTEST INFO | Saved log: Initializing canvas snapshot
[task 2024-06-28T11:05:42.362Z] 11:05:42     INFO - REFTEST INFO | Saved log: DoDrawWindow 0,0,800,1000
[task 2024-06-28T11:05:42.363Z] 11:05:42     INFO - REFTEST INFO | Saved log: [CONTENT] RecordResult fired
[task 2024-06-28T11:05:42.364Z] 11:05:42     INFO - REFTEST INFO | Saved log: RecordResult fired
[task 2024-06-28T11:05:42.365Z] 11:05:42     INFO - REFTEST INFO | Saved log: START file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/writing-mode/1083848-1-inline-border-ref.html
[task 2024-06-28T11:05:42.366Z] 11:05:42     INFO - REFTEST INFO | Saved log: [CONTENT] OnDocumentLoad triggering AfterOnLoadScripts
[task 2024-06-28T11:05:42.366Z] 11:05:42     INFO - REFTEST INFO | Saved log: Initializing canvas snapshot
[task 2024-06-28T11:05:42.371Z] 11:05:42     INFO - REFTEST INFO | Saved log: DoDrawWindow 0,0,800,1000
[task 2024-06-28T11:05:42.372Z] 11:05:42     INFO - REFTEST INFO | Saved log: [CONTENT] RecordResult fired
[task 2024-06-28T11:05:42.373Z] 11:05:42     INFO - REFTEST INFO | Saved log: RecordResult fired
[task 2024-06-28T11:05:42.384Z] 11:05:42     INFO - REFTEST TEST-END | layout/reftests/writing-mode/1083848-1-inline-border.html == layout/reftests/writing-mode/1083848-1-inline-border-ref.html
[task 2024-06-28T11:05:42.385Z] 11:05:42     INFO - REFTEST TEST-START | layout/reftests/writing-mode/1083848-2-inline-background.html == layout/reftests/writing-mode/1083848-2-inline-background-ref.html

Comment 1

[Jonathan Kew [:jfkthame]]

Ah, right, that test is dependent on the pref setting, so we need to add that to the manifest. I'll put up a patch in a while (out of the office today).

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1905330 - Remove letter-spacing from writing-mode inline-border reftest. r=#layout

It's not relevant to what the test is looking at.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c66500c2b1bf
Remove letter-spacing from writing-mode inline-border reftest. r=layout-reviewers,emilio

Comment 4

[Serban Stanca [:SerbanS]]

Patch doesn't seem to work in today's central as beta simulation.

Comment 5

[Aron Cseh]

https://hg.mozilla.org/mozilla-central/rev/c66500c2b1bf

Comment 6

[Cosmin Sabou [:CosminS]]

No longer happens in beta-sims.

Comment 7

[Serban Stanca [:SerbanS]]

This looks to happen only on late-beta simulation now.

Hi Jonathan! Could you please take another look?

Thank you!

Comment 8

[Daniel Holbert [:dholbert]]

We probably need a pref-flip (setting layout.css.letter-spacing.model to 2) for this reftest, I imagine? jfkthame, it sounds like that's what you were planning in comment 1, but you ended up opting to remove letter-spacing from the test instead. But some letter-spacing actually remained -- I see .h { writing-mode:horizontal-tb; letter-spacing: 0.4em } in the remaining contextual code.

This test presumably started passing in early-beta sims (comment 6) not due to the test-tweak that landed here, but rather due to the fact that we enabled this symmetrical letter-spacing behavior on early-beta on July 1st in bug 1905213.

Comment 9

[Jonathan Kew [:jfkthame]]

Yeah - I overlooked the letter-spacing on the .h class. Removing that and adjusting the width of the element avoids the issue; I meant to post this patch yesterday but I see I got distracted....

Comment 10

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1905330 followup - also remove letter-spacing from the horizontal testcase. r=#layout

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a92b779008c8
followup - also remove letter-spacing from the horizontal testcase. r=layout-reviewers,emilio

Comment 12

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

Set release status flags based on info from the regressing bug 1892262

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/a92b779008c8

Comment 14

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

The patch landed in nightly and beta is affected.
:jfkthame, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox129 to wontfix.

For more information, please visit BugBot documentation.

Comment 15

[Donal Meehan [:dmeehan]]

test-only change so doesn't need a formal uplift request.
I can take this in my next push to beta.

Comment 16

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1905330 followup - also remove letter-spacing from the horizontal testcase.

Original Revision: https://phabricator.services.mozilla.com/D216023

Comment 17

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: No user impact; late-beta reftest failure in CI
• Code covered by automated testing: yes
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: minimal
• Explanation of risk level: test-only patch, no code change
• String changes made/needed: none
• Is Android affected?: yes

Comment 18

[Jonathan Kew [:jfkthame]]

(In reply to Donal Meehan [:dmeehan] from comment #15)

test-only change so doesn't need a formal uplift request.
I can take this in my next push to beta.

Hah, thanks - I just saw this after completing the uplift form!

Comment 19

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/99354190e3ac

Comment 20

[Sandor Molnar[:smolnar]]

Verified fixed in today's late beta-sim