Allow panels to correctly resize toward bottom start

NEW
Unassigned

Status

()

Core
Widget
7 years ago
a year ago

People

(Reporter: mak, Unassigned)

Tracking

(Blocks: 1 bug, {meta})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Created attachment 527765 [details]
panel.xul

This is probably going to group a bunch of bugs, as a meta, but I think it's worth collecting the brokenness of panels attached to the top-right.

This prevents making the bookmarking panel resizeable and possibly other uses where the panel appears in the right (end actually for RTL) part of the screen.

Some of the issues:
- the flexible content pushes the panel till it fits, it should wrap instead
- width, minwidth, maxwidth (and same for height) don't work properly
- <resizer> is missing a bottomstart direction, and native theming is flipped (at least on Win)
- trying to resize does not resize content, and moves panel out of its anchor

I think there are other bugs around this thing, but fixing these would allow to easily workaround minor things.
Attachment #527765 - Attachment is patch: false
Attachment #527765 - Attachment mime type: text/plain → application/vnd.mozilla.xul+xml
Blocks: 418864

Comment 1

7 years ago
(In reply to comment #0)
> - width, minwidth, maxwidth (and same for height) don't work properly

This is bug 357725.


> - <resizer> is missing a bottomstart direction, and native theming is
> flipped (at least on Win)

There isn't such a value, or are you asking for it to be added?


> - trying to resize does not resize content, and moves panel out of its anchor

Probably also bug 357725.
(In reply to comment #1)
> There isn't such a value, or are you asking for it to be added?

Sorry I missed this message, I'm asking for it to be added.
Neil, I tested the current patch in bug 357725, but doesn't seem to make any difference on this test, it still behaves the same wrong way.

Shouldn't nsMenuPopupFrame::MoveTo check IsAnchored() before overriding mScreenXPos and mScreenYPos? Maybe using setters for those and assert if code changes them when the panel is anchored, may help debugging?
Depends on: 672254
Depends on: 674578
Created attachment 548871 [details] [diff] [review]
test patch for bookmarks panel

Attaching to allow testing the bookmarks panel, needs the patches in bug 672254 and bug 674578 (the patch in bug 357725 does not seem to make any difference for this case). Tested on Windows so far.
Current issues:
- when the panel reaches minwidth, it starts moving to the right, to a maximum of about 30 px (the panel has a 16px padding, that may be related). in rtl the panel becomes cut in the right.
- the arrow flickers when resizing the panel in ltr mode, does not flicker in rtl mode (annoying but minor issue)
Depends on: 674862
Created attachment 557917 [details]
button-in-panel.xul

this shows the arrow moving out of view when the panel reaches min-width, it is due to the min-width of the button, if you set a larger minwidth on the button the effect is even more visible. I guess may happen with other elements acting similar to the button.

Comment 6

7 years ago
Yes, that's a general issue, not specific to popups. The button's intrinsic minimum width is larger than the popup's minimum width, so the button's minimum size wins and the popup has to push its content out.

It could be overridden by setting min-width: 0 on the button.

Comment 7

4 years ago
Marco, Neil, out of interest, do you know if there's much/any desiry/urgency within mozilla to address this, or is it so far down the list of priorities as to be functionally out of sight?

Semi-related question: A great many people are interested in seeing this fixed, I'd say: divide the number of standard user posters here [https://bugzilla.mozilla.org/show_bug.cgi?id=418864] by the percentage of users who know enough to know when functionality is a bug, care enough to search about it, care more enough to find the bug page, care enough to sign up & log in & post & subscribe.
Given that, is there any scope to put a bounty on this? Is there any mechanism within mozilla's bugtracking system to track users' fix desires?

Cheers
(In reply to Simon Dedman from comment #7)
> Marco, Neil, out of interest, do you know if there's much/any desiry/urgency
> within mozilla to address this, or is it so far down the list of priorities
> as to be functionally out of sight?

no urgency. It should actually be feasible, something to define is how the panel should act when it gains more vertical space. Horizontal resize is not a big deal, but vertical resize should probably give additional value, like automatically expand folders picker when enough space is present.
Though the main issue is still the flicker expressed in comment 4. I didn't try applying the patch to a recent build. Someone might contribute here by updating and testing the patch.
ehr, clearly for this toolkit widget bug the only issue would be the flickering, any other stuff should end up in bug 418864.

Comment 10

4 years ago
Thanks Marco.

> any other stuff should end up in bug 418864.

I read through again and I couldn't see anything other than the resizing that really deserves to be within that bug. Lots of discussion, and a few "and also it should do this" but the chat has been going on for so long, many of those have been addresses. Subsequently - unless you saw something I didn't? - I feel that if the flickering is preventing the resizing, then the flickering could be the ONLY bug. Any other changes/improvements can come later (says he, Captain of Nothing. haha)


> flickering

Edit Bookmark Plus (https://addons.mozilla.org/en-US/firefox/addon/edit-bookmark-plus/) seems to work on windows, from memory, but flickering is AWFUL on linux with this. Not sure if that helps.


> I didn't try applying the patch to a recent build. Someone might contribute here by updating and testing the patch.
> applying to recent build
I'll give it a go. Dunno how but will google it.

> updating the patch
Did you have anything in mind specifically?

> testing the patch
Assumedly repeat the 'applying the patch' process I'll have googled in step 1?

Comment 11

4 years ago
addressed not addresses.

Comment 12

a year ago
The attachments to this bug are broken: "Remote XUL" not supported or somesuch.
The xul attachment was just to show a possible issue, for which comment 6 proposed a workaround. FWIW https://addons.mozilla.org/it/firefox/addon/remote-xul-manager/ should still work.
But what I wanted to point out was the test patch to the bookmarks panel, that would potentially still apply (with fixes).
You need to log in before you can comment on or make changes to this bug.