Closed
Bug 941196
Opened 11 years ago
Closed 11 years ago
Australis menu can jump around when opening long submenu
Categories
(Firefox :: Toolbars and Customization, defect)
Tracking
()
RESOLVED
FIXED
Firefox 30
People
(Reporter: Felipe, Assigned: mconley)
References
(Blocks 1 open bug, )
Details
(Whiteboard: [Australis:P3][good first verify])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Gijs
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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
Updated•11 years ago
|
Comment 1•11 years ago
|
||
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]
Assignee | ||
Updated•11 years ago
|
Whiteboard: [Australis:P2] → [Australis:P2][maybe-fix-on-aurora]
Assignee | ||
Updated•11 years ago
|
Whiteboard: [Australis:P2][maybe-fix-on-aurora] → [Australis:P2][can-fix-on-aurora]
Updated•11 years ago
|
Whiteboard: [Australis:P2][can-fix-on-aurora] → [Australis:P3][can-fix-on-aurora]
Assignee | ||
Comment 2•11 years ago
|
||
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)
Reporter | ||
Comment 3•11 years ago
|
||
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)
Comment 4•11 years ago
|
||
(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.)
Assignee | ||
Comment 5•11 years ago
|
||
Any thoughts on Gavin's suggestion, Neil?
Flags: needinfo?(enndeakin)
Comment 6•11 years ago
|
||
Does it work if you set panelUI.autoPosition = false on the panel when the history is opened?
Flags: needinfo?(enndeakin)
Assignee | ||
Comment 7•11 years ago
|
||
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)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → mconley
Assignee | ||
Comment 9•11 years ago
|
||
Thoughts on this, Jared?
Attachment #8372291 -
Flags: review?(jaws)
Reporter | ||
Comment 10•11 years ago
|
||
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.
Assignee | ||
Comment 11•11 years ago
|
||
Comment on attachment 8372291 [details] [diff] [review]
Patch v1
Ah, good call. Thanks Felipe!
Attachment #8372291 -
Flags: review?(jaws)
Comment 12•11 years ago
|
||
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.
Assignee | ||
Comment 13•11 years ago
|
||
Cool, ok, so maybe we'll just have the panelmultiview binding set that property automatically when it's switching to a subview.
Assignee | ||
Comment 14•11 years ago
|
||
Moving the autoPosition = false into the panelmultiview binding.
Attachment #8372291 -
Attachment is obsolete: true
Assignee | ||
Comment 15•11 years ago
|
||
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)
Comment 16•11 years ago
|
||
(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 17•11 years ago
|
||
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+
Assignee | ||
Comment 18•11 years ago
|
||
Need to test this on my Windows box.
Assignee | ||
Comment 19•11 years ago
|
||
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]
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P3][fixed-in-fx-team] → [Australis:P3]
Target Milestone: --- → Firefox 30
Updated•11 years ago
|
status-firefox29:
--- → fixed
status-firefox30:
--- → affected
tracking-firefox30:
--- → ?
Target Milestone: Firefox 30 → Firefox 29
Updated•11 years ago
|
Updated•11 years ago
|
Target Milestone: Firefox 29 → Firefox 30
Assignee | ||
Comment 21•11 years ago
|
||
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?
Updated•11 years ago
|
Attachment #8373507 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 22•11 years ago
|
||
Updated•11 years ago
|
Whiteboard: [Australis:P3] → [Australis:P3][good first verify]
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•