1705180 - [RTL] When editing a folder name (for an existing or new bookmarks folder) while adding a bookmark, the text field is too wide and the text gets clipped

Status: VERIFIED FIXED

(Toolkit Graveyard :: Notifications and Alerts, defect, P2)

Description

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Attachment: Screen Shot 2021-04-14 at 15.48.06.png

Affected platforms:
Platforms: Mac OS11

Preconditions
Start an RTL Firefox builds (i.e. ar)
Set the following prefs in about:config

• browser.proton.enabled = true
• prompts.windowPromptSubDialog = true
• prompts.contentPromptSubDialog = true
• browser.proton.modals.enabled = true

Steps to reproduce

1. Launch the Firefox browser in RTL build
2. visit any website
3. Click on bookmark icon
4. Expand folder by clicking on down arrow
5. Click on New folder option

Expected result
The field to input value should be properly displayed

Actual result
The field to input value is wrongly displayed

Comment 1

[:Gijs (he/him)]

This isn't really a proton issue - it reproduces on a January build without proton enabled, too. The root cause is that the coordinates we get from the XUL tree layout implementation are... creative, but don't really reflect reality too much. I filed bug 1708159 for that.

In particular, the code at https://searchfox.org/mozilla-central/rev/3aef835f6cb12e607154d56d68726767172571e4/toolkit/content/widgets/tree.js#1337-1346 ends up getting a "width diff" (which is meant to be the difference between the cell width and the text width) that is negative, which then means it sets too wide a width for the input field.

In an ideal world, the width should be such that the text shows up exactly where it was before the user started editing a field. We don't currently get the right information for that, and deep-diving into the XUL tree implementation that is 20 years old isn't really warranted right now, so I'm going to stopgap-fix this so at least the input doesn't end up wider than the folder tree.

Comment 2

[:Gijs (he/him)]

Attachment: Bug 1705180 - ensure tree editing doesn't exceed the bounds of the tree widget, r?jaws

As noted in https://bugzilla.mozilla.org/show_bug.cgi?id=1705180#c1, this is
not an ideal fix, but it should at least ensure the tree input field doesn't
get too wide and get clipped by the panel, with text disappearing.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f2196afeec59
ensure tree editing doesn't exceed the bounds of the tree widget, r=jaws

Comment 4

[Iulian Moraru]

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

Comment 5

[:Gijs (he/him)]

Comment on attachment 9218915 [details]
Bug 1705180 - ensure tree editing doesn't exceed the bounds of the tree widget, r?jaws

Beta/Release Uplift Approval Request

• User impact if declined: Required for MR1 / Proton
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: see comment 0
• List of other uplifts needed: n/a
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Minute JS changes in how we compute coordinates for tree editing fields in RTL mode, to ensure that we never exceed the bounds of the cell in which it appears.
• String changes made/needed: n/a

Comment 6

[Pascal Chevrel:pascalc]

Comment on attachment 9218915 [details]
Bug 1705180 - ensure tree editing doesn't exceed the bounds of the tree widget, r?jaws

Approved for 89 beta 6, thanks.

Comment 7

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/b271a0ca8059

Comment 8

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Attachment: nightly.png

Tested in latest Nightly 90.0a1 (2021-05-05) (64-bit) and Beta 89.0b8 (64-bit).
This issue is still reproducible in Nightly, attaching screenshot.
Best regards,
Clara.

Comment 9

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Attachment: beta.png

Beta has been fixed.

Comment 10

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Gijs, can you please take a look at this issue it seems that this issue still occurs in our latest nightly build, should I log a new issue ? or we can reopen this one ?

Comment 11

[:Gijs (he/him)]

You appear to be testing different cases on beta and on nightly. I think whether the issue reproduces depends on the nesting level of the folder you're adding/editing. Can you re-test?

Comment 12

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Attachment: beta vs nightly.png

Here, Gijs. Notice that for Nightly, that white folder is displayed, but in beta it isn't. (I used the EN version to make sure I'm standing in the same place, and not wihtin a folder inside of another folder). Let me know if this screenshot is clear.

Comment 13

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Attachment: white folder.webm

Here's beta EN build.

Comment 14

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Attachment: nightly folder.webm

This in Nightly EN build.

Comment 15

[:Gijs (he/him)]

The disappearing folder icon is bug 1702999, I think, which has been fixed.

The other issue appears related to whether or not a scrollbar is shown in the folder pane. I can reproduce the nightly issue if the scrollbar is shown, but not if the scrollbar is not shown (e.g. by collapsing more folders or removing them). I suspect this is also reproducible on beta, if you add enough folders so the folder pane shows a scrollbar. Clara, can you check that this is the case? If so, probably worth filing another bug at this point. :-\

Comment 16

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Attachment: Screen Shot 2021-05-13 at 12.33.27.png

Well, now it's not specific to RTL builds (but it is specific to macOS still)

I can reproduce in beta 89.0b11 (64-bit) but not in nightly 90.0a1 (2021-05-13) (64-bit)

Comment 17

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Attachment: Screen Shot 2021-05-13 at 12.33.46.png

Comment 18

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Should I file a new bug then ?

Clara

Comment 19

[:Gijs (he/him)]

(In reply to Clara Guerrero from comment #18)

Should I file a new bug then ?

Clara

Sure, let's get a separate bug on file.

Comment 20

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Done, please refer to https://bugzilla.mozilla.org/show_bug.cgi?id=1711891 thanks!
Updating these flags accordingly.
Best regards,
Clara