[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 Graveyard :: Notifications and Alerts, defect, P2)
Tracking
(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)
204.52 KB,
image/png
|
Details | |
48 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details | Review |
138.79 KB,
image/png
|
Details | |
1018.94 KB,
image/png
|
Details | |
106.94 KB,
image/png
|
Details | |
4.43 MB,
video/webm
|
Details | |
2.84 MB,
video/webm
|
Details | |
422.98 KB,
image/png
|
Details | |
683.76 KB,
image/png
|
Details |
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
- Launch the Firefox browser in RTL build
- visit any website
- Click on bookmark icon
- Expand folder by clicking on down arrow
- 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
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
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.
Assignee | ||
Comment 2•4 years ago
|
||
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 4•4 years ago
|
||
bugherder |
Assignee | ||
Comment 5•4 years ago
|
||
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
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Comment 6•4 years ago
|
||
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•4 years ago
|
||
bugherder uplift |
Updated•4 years ago
|
Reporter | ||
Comment 8•4 years ago
|
||
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.
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Comment 9•4 years ago
|
||
Beta has been fixed.
Reporter | ||
Comment 10•4 years ago
|
||
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 ?
Assignee | ||
Comment 11•4 years ago
|
||
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?
Reporter | ||
Comment 12•4 years ago
|
||
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.
Reporter | ||
Comment 13•4 years ago
|
||
Here's beta EN build.
Reporter | ||
Comment 14•4 years ago
|
||
This in Nightly EN build.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 15•4 years ago
|
||
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. :-\
Reporter | ||
Comment 16•4 years ago
•
|
||
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)
Reporter | ||
Comment 17•4 years ago
|
||
Reporter | ||
Comment 18•4 years ago
|
||
Should I file a new bug then ?
Clara
Assignee | ||
Comment 19•4 years ago
|
||
(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.
Reporter | ||
Comment 20•4 years ago
|
||
Done, please refer to https://bugzilla.mozilla.org/show_bug.cgi?id=1711891 thanks!
Updating these flags accordingly.
Best regards,
Clara
Reporter | ||
Updated•4 years ago
|
Updated•4 years ago
|
Updated•1 year ago
|
Description
•