In-content prefs : scrollbars appear in a wrong position

VERIFIED FIXED in Firefox 31

Status

()

defect
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: ge3k0s, Assigned: Paenglab)

Tracking

Trunk
Firefox 31
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox31 verified)

Details

(Whiteboard: p=0 s=it-31c-30a-29b.3 [qa!])

Attachments

(4 attachments)

The scrollbars appears in the "middle" of in-content preferences. There is no more UI to put them here(unlike in the add-ons manager), so they should appear on the right of the window like other classic scrollbars.
Blocks: 738796
Blocks: 718011
Summary: In-content prefs : scrollbars are wrongly positioned → In-content prefs : scrollbars appear in a wrong position
add screenshot of an example with horizontal and vertical scrollbar
The previous version had this width limit of 800px (and the scrollbars at the same position) and I took it over.

Should we still have a width limit? Now the elements are using more place as they are bigger. If the limit should stay, I can try to move the scrollbars to the edge.
Flags: needinfo?(mmaslaney)
FWIW see bug 738796 comment 73.
Flags: needinfo?(mmaslaney)
Status: UNCONFIRMED → NEW
Ever confirmed: true
I think the scrollbars should really behave like on other pages ie be on the edges of the window.
Flags: firefox-backlog+
Does anyone have a model of what the UI here should look like? The older in-content prefs had the tab contents in a box over the page. Here, the tab fills the entire page, with no box around it. I tried tweaking the CSS a bit but wasn't able to come up with something that looks nice.
Duplicate of this bug: 992474
Whiteboard: p=5
(In reply to Manish Goregaokar [:manishearth] from comment #5)
> Does anyone have a model of what the UI here should look like? The older
> in-content prefs had the tab contents in a box over the page. Here, the tab
> fills the entire page, with no box around it. I tried tweaking the CSS a bit
> but wasn't able to come up with something that looks nice.

Yeah, I am not sure if we can fix this with just CSS. I think the structure of the page needs to change.

It looks like the content is in a box inside of box [ux-yoDawg] Instead it should fill the whole right side of the pane.

Actually, something like this might work:

.main-content {
  max-width: auto;
  padding: 50px 40px 40px;
  overflow: auto;

}

prefpane > .content-box {
  overflow: auto;
}

prefpane {
  max-width: 800px;
  padding: 0;
  height: max-content;
  font-family: "Clear Sans", sans-serif;
  font-size: 1.25rem;
  line-height: 22px;
  color: #424E5A;
}

prefpane > .content-box {
  overflow: visible;
}
Keywords: uiwanted
Move the scrollbars to the edges.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8405598 - Flags: review?(jaws)
Comment on attachment 8405598 [details] [diff] [review]
inContentScrollbars.patch

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

::: browser/themes/shared/incontentprefs/preferences.css
@@ +32,5 @@
>    overflow: auto;
>  }
>  
>  prefpane {
> +  max-width: 800px;

Doesn't this just push this bug's issue to a different place?

@@ +41,5 @@
>    color: #424E5A;
>  }
>  
>  prefpane > .content-box {
> +  overflow: visible;

I don't see any .content-box children of prefpane's within the preferences. Can you point out where this is used? Nonetheless, 'visible' is the initial value of 'overflow', and so specifying it here is not needed. You could just delete this ruleset.
Attachment #8405598 - Flags: review?(jaws) → review-
Posted image prefs-DOM.png
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #9)
> Comment on attachment 8405598 [details] [diff] [review]
> inContentScrollbars.patch
> 
> Review of attachment 8405598 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/themes/shared/incontentprefs/preferences.css
> @@ +32,5 @@
> >    overflow: auto;
> >  }
> >  
> >  prefpane {
> > +  max-width: 800px;
> 
> Doesn't this just push this bug's issue to a different place?

This let's the prefpane have max-width of 800px but the scrollbars are at the window border as it should be. I see no issue with this patch.

Stephen leaved this width in his proposal and I adopted it. If you want I should remove it, I'll do it.

> @@ +41,5 @@
> >    color: #424E5A;
> >  }
> >  
> >  prefpane > .content-box {
> > +  overflow: visible;
> 
> I don't see any .content-box children of prefpane's within the preferences.
> Can you point out where this is used? Nonetheless, 'visible' is the initial
> value of 'overflow', and so specifying it here is not needed. You could just
> delete this ruleset.

See the screenshot of DOMi. And overflow: visible; is needed because of http://mxr.mozilla.org/mozilla-central/source/toolkit/content/xul.css#1173.
Comment on attachment 8405598 [details] [diff] [review]
inContentScrollbars.patch

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

My apologies, I should have used DOMi instead of the Inspector. The patch looks good, thanks.
Attachment #8405598 - Flags: review- → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/489aa6e7e740
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: p=5[fixed-in-fx-team] → p=5
Target Milestone: --- → Firefox 31
Whiteboard: p=5 → p=0 s=it-31c-30a-29b.3 [qa?]
Florin, please assign this for verification in the current iteration.
QA Contact: florin.mezei
Whiteboard: p=0 s=it-31c-30a-29b.3 [qa?] → p=0 s=it-31c-30a-29b.3 [qa+]
QA Contact: florin.mezei → camelia.badau
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0
Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Firefox/31.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:31.0) Gecko/20100101 Firefox/31.0

Verified fixed on latest Nightly 31.0a1 (buildID: 20140416030202).
Status: RESOLVED → VERIFIED
Whiteboard: p=0 s=it-31c-30a-29b.3 [qa+] → p=0 s=it-31c-30a-29b.3 [qa!]
You need to log in before you can comment on or make changes to this bug.