1805385 - The Callout Tour for pdf.js is wrongly positioned on Firefox RTL locale builds

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P1)

Description

[Cosmin Muntean [:cmuntean], Ecosystem QA]

Attachment: image_2022_12_13T09_22_40_065Z.png

[Affected versions]:

• Nightly 110.0b1
• Firefox Beta 109.0b1

[Affected Platforms]:

• Mac 12.4
• Windows 10 x64
• Linux Mint 20

[Prerequisites]:

• Have a Firefox RTL build extracted/installed (eg: ar build).
• Have the "nimbus.debug" set to true in about:config.
• Be enrolled in any of the experiment branches. You can force enroll by navigating to the following links: Treatment A or Treatment B.

[Steps to reproduce]:

1. Open the Firefox browser with the profile from the prerequisites.
2. Navigate to a PDF link, eg: TestPDFFile.
3. Observe the Callout Tour for pdf.js.

[Expected result]:

• The Callout Tour for pdf.js is positioned in the left part pointing to the Text icon.

[Actual result]:

• The Callout Tour for pdf.js is positioned in the right part pointing to nothing in the toolbar.

[Notes]:

• The issue is also reproducible for the second screen.
• Attached is a screenshot of the issue.

Comment 1

[Meg Viar [:mviar]]

We'll need to land a means of overriding the reversal of the arrow position for RTL layouts or do so by default when using a position override.

To unblock the 109 experiment, we could target only locales using LTR layouts.

Comment 2

[Meg Viar [:mviar]]

Does the above sound like a reasonable alternative @vtay? Or should we consider this an uplift candidate?

Comment 3

[Jason Prickett]

Attachment: Bug 1805385 - Flip direction of absolutely positioned feature callout in RTL layouts r=mviar,#omc-reviewers

Comment 4

[Meg Viar [:mviar]]

Attachment: Bug 1805385 - Ignore direction of page layout when using custom position overrides in Feature Callouts

Comment 5

[Vtay]

As discussed, let's see if there is a low effort uplift for this. The alternative plan is to target only locales using LTR layouts.

Comment 6

[Meg Viar [:mviar]]

@jprickett should I assign this to you since you already have a patch?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9010e3c9421e
Flip direction of absolutely positioned feature callout in RTL layouts r=mviar

Comment 8

[Cristian Tuns]

Backed out for causing mochitest failures

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/components/firefoxview/tests/browser/browser_feature_callout_position.js | Feature callout container has a top position of 500, and left position of 500 -

bc log: https://treeherder.mozilla.org/logviewer?job_id=400437847&repo=autoland

Comment 9

[Jason Prickett]

That test was for functionality that my patch changed and is covered in new tests. I'll remove it and reland.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5096e8be5773
Flip direction of absolutely positioned feature callout in RTL layouts r=mviar

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/5096e8be5773

Comment 12

[Meg Viar [:mviar]]

Comment on attachment 9308091 [details]
Bug 1805385 - Flip direction of absolutely positioned feature callout in RTL layouts r=mviar,#omc-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Users on builds with right to left languages will see the feature tour for PDF.js annotation in the wrong position. This could result in confusion or frustration.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: On a build with a RTL language such as Arabic:

• Navigate to about:config and set the browser.pdfjs.feature-tour pref to {"screen":"FEATURE_CALLOUT_1_A","complete":false}
• Navigate to any PDF file (https://upload.wikimedia.org/wikipedia/commons/d/d3/Test.pdf)
• Note that the feature callout appears on the right side of the screen and has an arrow that points to the appropriate element
• Install the lang pack into your nightly build by dragging the langpack file into the nightly window and approve the addon installation
• Navigate to the Firefox's settings page, and type "lang" into the search, then select your newly installed RTL language from the language dropdown list
• Reload the browser with CTRL+ALT+R in windows/linux, or CMD+OPT+R in MacOS. Note: The browser must be reloaded after the language is switched, or newly opened PDF files (including PDF.js) will not render in RTL mode appropriately.
• Navigate to any PDF file (https://upload.wikimedia.org/wikipedia/commons/d/d3/Test.pdf)
• Note that the feature callout appears on the left side of the screen and has an arrow that points to the appropriate element

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This change is limited to the Feature Callout surface and only alters the appearance of the callout messages for users with RTL language layouts.
• String changes made/needed: n/a
• Is Android affected?: No

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9308091 [details]
Bug 1805385 - Flip direction of absolutely positioned feature callout in RTL layouts r=mviar,#omc-reviewers

Approved for 109.0b8.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3002fea267ad

Comment 15

[Cosmin Muntean [:cmuntean], Ecosystem QA]

I have verified this issue on the latest Nightly 110.0a1 ar build (Build ID: 20230104163752) and the latest Beta 109.0b8 ar build (Build ID: 20230103185821) on Windows 10 x64, macOS 12.4 and Linux Mint 20.

• The Callout Tour for pdf.js is correctly positioned in both screens.