Closed
Bug 1368317
Opened 8 years ago
Closed 7 years ago
Sidebar splitter overlaps web content when moving the sidebar to the right
Categories
(Firefox :: General, defect, P1)
Firefox
General
Tracking
()
People
(Reporter: dao, Assigned: Gijs)
References
Details
(Whiteboard: [photon-structure])
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
bgrins
:
review+
jcristau
:
approval-mozilla-beta+
|
Details |
The sidebar splitter is rendered 1px wide but actually wider to be a viable mouse target. The transparent part of the splitter is supposed to overlap the sidebar rather than the content area, as making part of the web content inaccessible is a no-go.
The splitter being on the right is supposed to be handled by this code:
http://searchfox.org/mozilla-central/rev/596d188f6dbc8cb023e625f0a4310d184875f8fc/browser/themes/shared/sidebar.inc.css#32-37
Since bug 1355331 used the ordinal attribute to move the sidebar, this doesn't work. I would again suggest moving the DOM nodes instead. This has the added benefit of removing a dependency on special XUL features that we may want to get rid of some day.
Updated•7 years ago
|
Flags: qe-verify?
Priority: -- → P2
QA Contact: gwimberly
Whiteboard: [photon-structure][triage] → [photon-structure]
Comment 1•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #0)
> Since bug 1355331 used the ordinal attribute to move the sidebar, this
> doesn't work. I would again suggest moving the DOM nodes instead. This has
> the added benefit of removing a dependency on special XUL features that we
> may want to get rid of some day.
We can use the CSS 'order' property if this is converted to CSS flexbox. Moving the DOM node would cause the <browser> in the sidebar to reload and lose the user's state
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 56.1 - Jun 26
Priority: P2 → P1
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8877572 [details]
Bug 1368317 - fix sidebar overlap styling for when the sidebar is on the end-side of the window (rhs in ltr),
https://reviewboard.mozilla.org/r/149050/#review153554
::: browser/themes/shared/sidebar.inc.css:29
(Diff revision 1)
> margin-inline-start: -3px;
> position: relative;
> }
>
> +.sidebar-splitter[overlapend],
> #appcontent ~ .sidebar-splitter {
Is there a time when `#appcontent ~ .sidebar-splitter` is true? This was copied in from the various browser.css files in https://bugzilla.mozilla.org/show_bug.cgi?id=1355324 but I'm not sure that either appcontent or the splitter is moved in the DOM.
Attachment #8877572 -
Flags: review?(bgrinstead) → review+
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #3)
> Comment on attachment 8877572 [details]
> Bug 1368317 - fix sidebar overlap styling for when the sidebar is on the
> end-side of the window (rhs in ltr),
>
> https://reviewboard.mozilla.org/r/149050/#review153554
>
> ::: browser/themes/shared/sidebar.inc.css:29
> (Diff revision 1)
> > margin-inline-start: -3px;
> > position: relative;
> > }
> >
> > +.sidebar-splitter[overlapend],
> > #appcontent ~ .sidebar-splitter {
>
> Is there a time when `#appcontent ~ .sidebar-splitter` is true? This was
> copied in from the various browser.css files in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1355324 but I'm not sure that
> either appcontent or the splitter is moved in the DOM.
AFAICT this was for social sidebars and they were removed in bug 1289549. I'll remove that part of the selector.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8877572 [details]
Bug 1368317 - fix sidebar overlap styling for when the sidebar is on the end-side of the window (rhs in ltr),
https://reviewboard.mozilla.org/r/149050/#review153956
Attachment #8877572 -
Flags: review+
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8877572 [details]
Bug 1368317 - fix sidebar overlap styling for when the sidebar is on the end-side of the window (rhs in ltr),
https://reviewboard.mozilla.org/r/149050/#review153958
Big green buttons aren't always the ones I want. :-(
Attachment #8877572 -
Flags: review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/603c076c05e5
fix sidebar overlap styling for when the sidebar is on the end-side of the window (rhs in ltr), r=bgrins
Comment 9•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Assignee | ||
Comment 10•7 years ago
|
||
Comment on attachment 8877572 [details]
Bug 1368317 - fix sidebar overlap styling for when the sidebar is on the end-side of the window (rhs in ltr),
Approval Request Comment
[Feature/Bug causing the regression]: bug 1355331
[User impact if declined]: part of the website can't be clicked or interacted with using the mouse when the sidebar is moved to the right
[Is this code covered by automated tests?]: no, styling issue
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]:
1. open firefox
2. open sidebar (e.g. hit accel-b to open the bookmarks sidebar)
3. using the menu in the sidebar header, move the sidebar to the right
4. hover/click the mouse next to the edge of the sidebar. The website should remain responsive right up to the edge of the sidebar.
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: very simple styling change
[String changes made/needed]: nope
Attachment #8877572 -
Flags: approval-mozilla-beta?
Assignee | ||
Updated•7 years ago
|
Flags: qe-verify? → qe-verify+
Assignee | ||
Updated•7 years ago
|
status-firefox55:
--- → affected
Comment 11•7 years ago
|
||
Comment on attachment 8877572 [details]
Bug 1368317 - fix sidebar overlap styling for when the sidebar is on the end-side of the window (rhs in ltr),
sidebar styling fix, beta55+
Attachment #8877572 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 12•7 years ago
|
||
bugherder uplift |
Comment 13•7 years ago
|
||
I verified this as fixed as suggested in comment 10. Latest nightly is used for testing.
Updating the bug accordingly.
Status: RESOLVED → VERIFIED
Updated•7 years ago
|
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•