Closed
Bug 1335705
Opened 7 years ago
Closed 7 years ago
GTK 3.20: Incorrect render <input type=range> - no thumb
Categories
(Core :: Widget: Gtk, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: striptm, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: nightly-community, Whiteboard: tpi:+)
Attachments
(6 files, 2 obsolete files)
In Ubuntu 16.10 with themes Radiance and Ambiance, the <input type=range> controls is not displayed correctly. A rectangle is displayed but the slider (drag control) is not present. On another computer with Ubuntu 16.04 is correct.
Reporter | ||
Comment 1•7 years ago
|
||
Reporter | ||
Comment 2•7 years ago
|
||
Comment 3•7 years ago
|
||
Updated•7 years ago
|
Attachment #8832453 -
Attachment mime type: text/plain → text/html
Comment 4•7 years ago
|
||
Attachment #8832453 -
Attachment is obsolete: true
Updated•7 years ago
|
Attachment #8832454 -
Attachment mime type: text/plain → text/html
Comment 5•7 years ago
|
||
Confirmed by another 16.10 user, I think we use native GTK widgets for this and 16.10 uses GTK 3.20 vs 3.18 for 16.04, so CCing people with Linux skills.
Keywords: nightly-community
Comment 6•7 years ago
|
||
That's because MOZ_GTK_SCALE_HORIZONTAL has "min-width" style property 0 and we rely on that to derive slider min size. We may check that value from some minimal size.
Updated•7 years ago
|
Component: Layout: Form Controls → Widget: Gtk
Comment 7•7 years ago
|
||
Well, looks like we need to fix scale rendering completely on Gtk > 3.20. Looking into it.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Attachment #8833304 -
Attachment is obsolete: true
Attachment #8833304 -
Flags: review?(karlt)
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8833304 [details] Bug 1335705 - Consider also margin when determine range widget slider size, https://reviewboard.mozilla.org/r/109564/#review111032
Updated•7 years ago
|
Attachment #8833304 -
Attachment is obsolete: false
Comment 11•7 years ago
|
||
Comment on attachment 8833304 [details] Bug 1335705 - Consider also margin when determine range widget slider size, Looks like another review request discarded this one. Trying to restore, if that does not work I'll submit another one.
Attachment #8833304 -
Flags: review?(karlt)
Updated•7 years ago
|
Priority: -- → P2
Whiteboard: tpi:+
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8833306 [details] Bug 1335705 - Consider also margin when determine range widget slider size, https://reviewboard.mozilla.org/r/109570/#review113998 This is pretty much what moz_gtk_scale_paint() wants, thanks. I'm not sure whether or not nsNativeThemeGTK::GetMinimumWidgetSize() should also consider the scale widget minsize. Thank you for using mozreview. It makes review much easier. ::: widget/gtk/gtk3drawing.cpp:2504 (Diff revision 1) > gtk_style_context_get_padding(style, GTK_STATE_FLAG_NORMAL, &padding); > > *height += (border.top + border.bottom + padding.top + padding.bottom); > ReleaseStyleContext(style); > } > > Bug 1335705 - For Gtk > 3.20 determine scale through size from trough CSS node and also add border/padding/margin, r?karlt ITYM "scale trough size".
Attachment #8833306 -
Flags: review?(karlt) → review+
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8833304 [details] Bug 1335705 - Consider also margin when determine range widget slider size, https://reviewboard.mozilla.org/r/109564/#review113980 Took me a while to understand that the negative margin affects the minimum border box size because the margin box cannot be less than zero, but looks good thanks. ::: widget/gtk/gtk3drawing.cpp:2564 (Diff revision 1) > gtk_style_context_get(style, gtk_style_context_get_state(style), > - "min-width", thumb_length, > - "min-height", thumb_height, > + "min-width", &min_width, > + "min-height", &min_height, > nullptr); > + GtkBorder margin; > + gtk_style_context_get_margin(style, gtk_style_context_get_state(style), Please store and use the result of the previous gtk_style_context_get_state(style) call instead of calling again. ::: widget/gtk/gtk3drawing.cpp:2571 (Diff revision 1) > + if (margin_width < 0 && min_width < - margin_width) > + min_width = - margin_width; > + if (margin_height < 0 && min_height < - margin_height) > + min_height = - margin_height; Gecko style is no space between unary operators ('-' here) and their operands. No need for the margin_width/height < 0 checks, as the max of min_width and -margin_width is sufficient with gint arithmetic.
Comment 15•7 years ago
|
||
Comment on attachment 8833304 [details] Bug 1335705 - Consider also margin when determine range widget slider size, (In reply to Martin Stránský from comment #11) > Looks like another review request discarded this one. Trying to restore, if > that does not work I'll submit another one. All patches for a bug need to be push together or mozreview assumes that patches not pushed again have been discarded. I assume that is what happened at least. It is possible to preview changes before publishing. I use this in ~/.hgrc so that I don't need to reply to the prompt and publish after preview. [reviewboard] autopublish = False r+ thanks with issues in reviewboard addressed.
Attachment #8833304 -
Flags: review?(karlt) → review+
Comment 16•7 years ago
|
||
(In reply to Karl Tomlinson (:karlt) from comment #13) > I'm not sure > whether or not nsNativeThemeGTK::GetMinimumWidgetSize() should also consider > the scale widget minsize. I think nsRangeFrame::GetMinISize() is depending on GetMinimumWidgetSize() to return the minimum size of the whole range (scale widget), so that extending thumbs don't overlap other content, but that's a lesser bug than this one. http://searchfox.org/mozilla-central/rev/cac6cb6a10afb8ebb2ecfbeeedaff7c66f57dd75/layout/forms/nsRangeFrame.cpp#809
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 20•7 years ago
|
||
(In reply to Karl Tomlinson (:karlt) from comment #15) > All patches for a bug need to be push together or mozreview assumes that > patches not pushed again have been discarded. I assume that is what > happened at least. I think I mixed patches for more bugs together which confuses mozreview. Thanks for the hint I'll try that.
Comment 21•7 years ago
|
||
mozreview-review |
Comment on attachment 8839855 [details] Bug 1335705 - For Gtk > 3.20 determine scale trough size from trough CSS node and also add border/padding/margin, https://reviewboard.mozilla.org/r/114444/#review115886 ::: widget/gtk/gtk3drawing.cpp:2508 (Diff revision 1) > } > > void > moz_gtk_get_scale_metrics(GtkOrientation orient, gint* scale_width, > gint* scale_height) > { This is the same patch as the previous one but with fixed description. Mozreview seems to lose track because of missing mozreview ID.
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8839855 [details] Bug 1335705 - For Gtk > 3.20 determine scale trough size from trough CSS node and also add border/padding/margin, https://reviewboard.mozilla.org/r/114444/#review116202
Attachment #8839855 -
Flags: review?(karlt) → review+
Comment 23•7 years ago
|
||
mozreview-review |
Comment on attachment 8833306 [details] Bug 1335705 - Consider also margin when determine range widget slider size, https://reviewboard.mozilla.org/r/109570/#review116200 ::: widget/gtk/gtk3drawing.cpp:2571 (Diff revision 2) > + if (margin_width < 0 && min_width < - margin_width) > + min_width = -margin_width; > + if (margin_height < 0 && min_height < - margin_height) Gecko style is no space between unary operators ('-' here) and their operands margin_width/height (in the boolean condition for the if block too). No need for the margin_width/height < 0 checks, as the max of min_width and -margin_width is sufficient with gint arithmetic.
Comment 24•7 years ago
|
||
(In reply to Martin Stránský from comment #21) > Mozreview seems to lose track because of missing mozreview ID. Yes. I suspect it uses some heuristics when there is no mozreview ID, but I guess differing number of patches and differing commit messages confuse those.
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Attachment #8839855 -
Attachment is obsolete: true
Comment 26•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8833306 [details] Bug 1335705 - Consider also margin when determine range widget slider size, https://reviewboard.mozilla.org/r/109570/#review116200 > Gecko style is no space between unary operators ('-' here) and their operands margin_width/height (in the boolean condition for the if block too). > > No need for the margin_width/height < 0 checks, as the max of min_width and -margin_width is sufficient with gint arithmetic. Sorry I overlooked the "-" in the condition.
Updated•7 years ago
|
Attachment #8839855 -
Attachment is obsolete: false
Updated•7 years ago
|
Attachment #8833304 -
Attachment is obsolete: true
Updated•7 years ago
|
Keywords: checkin-needed
Comment 27•7 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2c2b324b9afb Consider also margin when determine range widget slider size, r=karlt
Keywords: checkin-needed
Comment 28•7 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/41a156cf149e For Gtk > 3.20 determine scale trough size from trough CSS node and also add border/padding/margin, r=karlt
Comment 29•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2c2b324b9afb https://hg.mozilla.org/mozilla-central/rev/41a156cf149e
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Updated•7 years ago
|
Summary: Incorrect render <input type=range> → GTK 3.20: Incorrect render <input type=range> - no thumb
You need to log in
before you can comment on or make changes to this bug.
Description
•