Closed Bug 1705180 Opened 5 months ago Closed 5 months ago

[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

Categories

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

Desktop
All
defect

Tracking

()

VERIFIED FIXED
90 Branch
Tracking Status
firefox-esr78 --- wontfix
firefox87 --- wontfix
firefox88 --- wontfix
firefox89 --- verified
firefox90 --- verified

People

(Reporter: clara.guerrero, Assigned: Gijs)

References

(Blocks 3 open bugs)

Details

(Keywords: rtl, Whiteboard: [proton-modals] [proton-door-hangers] [priority:2a] [proton-uplift])

Attachments

(9 files)

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

Severity: -- → S3
Has Regression Range: --- → no
Has STR: --- → yes
Summary: When adding new folder in RTL build, the field to input value is wrongly displayed → When adding new Bookmark folder in RTL build, the field to input value is wrongly displayed
Whiteboard: [proton-modals][proton-door-hangers]
Priority: -- → P2
Whiteboard: [proton-modals][proton-door-hangers] → [proton-modals] [proton-door-hangers] [priority:2a]
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
See Also: → 1708159

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.

Keywords: rtl
OS: Unspecified → All
Hardware: Unspecified → Desktop
Summary: When adding new Bookmark folder in RTL build, the field to input value is wrongly displayed → [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

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.

Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f2196afeec59
ensure tree editing doesn't exceed the bounds of the tree widget, r=jaws
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

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
Attachment #9218915 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Whiteboard: [proton-modals] [proton-door-hangers] [priority:2a] → [proton-modals] [proton-door-hangers] [priority:2a] [proton-uplift]

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.

Attachment #9218915 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]
Attached image 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.

Flags: needinfo?(gijskruitbosch+bugs)
Attached image beta.png

Beta has been fixed.

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 ?

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?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(clara.guerrero)
Attached image 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.

Flags: needinfo?(clara.guerrero)
Attached video white folder.webm

Here's beta EN build.

Attached video nightly folder.webm

This in Nightly EN build.

Flags: needinfo?(gijskruitbosch+bugs)

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. :-\

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(clara.guerrero)

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)

Flags: needinfo?(clara.guerrero)

Should I file a new bug then ?

Clara

Flags: needinfo?(gijskruitbosch+bugs)

(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.

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(clara.guerrero)

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

Flags: needinfo?(clara.guerrero)
Status: RESOLVED → VERIFIED
See Also: → 1711891
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.