Closed Bug 941196 Opened 6 years ago Closed 6 years ago

Australis menu can jump around when opening long submenu

Categories

(Firefox :: Toolbars and Customization, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 30
Tracking Status
firefox29 --- fixed
firefox30 --- fixed

People

(Reporter: Felipe, Assigned: mconley)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [Australis:P3][good first verify])

Attachments

(1 file, 1 obsolete file)

When opening a long submenu like history, the entire panel will shift upwards a bit.

Also, depending on the position at the screen, the panel can flip to the "open upwards" position (but the arrow is still at the top)

Both probs can be viewed at this screencast: http://screencast.com/t/h0VKit5qOR
Blocks: australis-cust
No longer blocks: australis-merge
I'm not sure this isn't a dupe, but let's at least make this a P2 to figure out why/how this is happening.
Whiteboard: [Australis:P2]
Whiteboard: [Australis:P2] → [Australis:P2][maybe-fix-on-aurora]
Whiteboard: [Australis:P2][maybe-fix-on-aurora] → [Australis:P2][can-fix-on-aurora]
Whiteboard: [Australis:P2][can-fix-on-aurora] → [Australis:P3][can-fix-on-aurora]
I believe the cause for this bug may have been fixed by the fix in bug 941051.

Felipe - are you still able to reproduce? Because I'm not.
Flags: needinfo?(felipc)
I cannot reproduce the first part of the bug anymore. That does seem fixed by bug 941051.

I can still reproduce the second part, i.e., the menu can switch to the "open upwards" position.  But that requires the menu to be initially opened in a very specific Y position of the screen, I'd say there's only 10px where the bug can be triggered:  the main menu pane must fit in the screen underneath the button, but the height stretch caused by opening a sub-pane will flip it to the other position.
Flags: needinfo?(felipc)
(In reply to :Felipe Gomes from comment #3)
> I can still reproduce the second part, i.e., the menu can switch to the
> "open upwards" position.  But that requires the menu to be initially opened
> in a very specific Y position of the screen, I'd say there's only 10px where
> the bug can be triggered:  the main menu pane must fit in the screen
> underneath the button, but the height stretch caused by opening a sub-pane
> will flip it to the other position.

This particular bug seems much less serious. It seems like one way to address this would be to ensure that the panel always opens in the direction that has the most screen space, regardless of whether it will fit on the "best" side?

(I'm sure there's some problem with that solution that I'm missing, given what I know of panel-positioning trickiness.)
Any thoughts on Gavin's suggestion, Neil?
Flags: needinfo?(enndeakin)
Does it work if you set panelUI.autoPosition = false on the panel when the history is opened?
Flags: needinfo?(enndeakin)
Oh, yes, this seems to resolve the issue for me! Felipe, can you confirm? (You can get easy access to the panel element property via PanelUI.panel.autoPosition from the Browser Console).
Flags: needinfo?(felipc)
Indeed, it fixes it!
Flags: needinfo?(felipc)
Assignee: nobody → mconley
Attached patch Patch v1 (obsolete) — Splinter Review
Thoughts on this, Jared?
Attachment #8372291 - Flags: review?(jaws)
Comment on attachment 8372291 [details] [diff] [review]
Patch v1

FWIW, on my experimentation with Scratchpad, panel.autoPosition gets reset to true every time the panel closes. So I think that you'll need to set it when the panel is open, or when a subview is gonna be clicked, instead of in the init code.
Comment on attachment 8372291 [details] [diff] [review]
Patch v1

Ah, good call. Thanks Felipe!
Attachment #8372291 - Flags: review?(jaws)
Yes, autoPosition gets reset when a request is made to open a popup so that the popup can get positioned properly.

You may want to only to only have it set while the history is open, as I don't know what other kind of manipulation might need the popup to move while it is open.
Cool, ok, so maybe we'll just have the panelmultiview binding set that property automatically when it's switching to a subview.
Attached patch Patch v2Splinter Review
Moving the autoPosition = false into the panelmultiview binding.
Attachment #8372291 - Attachment is obsolete: true
Comment on attachment 8373507 [details] [diff] [review]
Patch v2

I _think_ we want to set this every time the panel opens. If I set it to only when showing a subview, it seems like the panel can stretch off screen, which isn't good...
Attachment #8373507 - Flags: review?(gijskruitbosch+bugs)
(In reply to Mike Conley (:mconley) from comment #15)
> Comment on attachment 8373507 [details] [diff] [review]
> Patch v2
> 
> I _think_ we want to set this every time the panel opens. If I set it to
> only when showing a subview, it seems like the panel can stretch off screen,
> which isn't good...

I'm so confused. Does this mean that despite bug 627974 we'd expand the panel underneath the taskbar? That doesn't seem OK...
Comment on attachment 8373507 [details] [diff] [review]
Patch v2

Review of attachment 8373507 [details] [diff] [review]:
-----------------------------------------------------------------

r+ on the condition that this doesn't regress the aforementioned. Otherwise we should do more work here...
Attachment #8373507 - Flags: review?(gijskruitbosch+bugs) → review+
Need to test this on my Windows box.
Tested, and this does fix the problem, and does not appear to regress our tendency to not put the panel under the task bar. If the double-negatives are confusing, basically I think we're good here.

remote:   https://hg.mozilla.org/integration/fx-team/rev/729c53d0ed7e

Gonna let this bake on m-c before I request uplift.
Whiteboard: [Australis:P3][can-fix-on-aurora] → [Australis:P3][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/729c53d0ed7e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P3][fixed-in-fx-team] → [Australis:P3]
Target Milestone: --- → Firefox 30
Target Milestone: Firefox 30 → Firefox 29
Target Milestone: Firefox 29 → Firefox 30
Comment on attachment 8373507 [details] [diff] [review]
Patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 

Australis.


User impact if declined: 

If a user has their window in restored mode, and positioned so that their menu button is somewhere near the vertical center of their screen, after opening the menu panel, opening a long subview can cause the panel to flip orientations, which is jarring.


Testing completed (on m-c, etc.): 

This has baked for a few days on m-c.


Risk to taking this patch (and alternatives if risky): 

Low risk.


String or IDL/UUID changes made by this patch:

None.
Attachment #8373507 - Flags: approval-mozilla-aurora?
Attachment #8373507 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [Australis:P3] → [Australis:P3][good first verify]
See Also: → 1520607
You need to log in before you can comment on or make changes to this bug.