Closed
Bug 365243
Opened 18 years ago
Closed 17 years ago
use small native scrollbars for HTML form elements
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jaas, Assigned: jaas)
References
Details
Attachments
(4 files, 3 obsolete files)
12.51 KB,
image/jpeg
|
Details | |
24.80 KB,
image/png
|
Details | |
51.22 KB,
image/png
|
Details | |
10.14 KB,
patch
|
roc
:
superreview+
|
Details | Diff | Splinter Review |
We should use small native scrollbars for HTML form elements. That would be more efficient space-wise and it'd look better.
This works for Firefox only. It isn't the right way to do it, but it is a start.
Attachment #249866 -
Attachment is patch: false
Attachment #249866 -
Attachment mime type: text/plain → image/jpeg
This should work for all products, including Camino.
Attachment #249865 -
Attachment is obsolete: true
Attachment #249880 -
Flags: review?
Attachment #249880 -
Flags: review? → review?(stuart.morgan)
Comment 4•18 years ago
|
||
Comment on attachment 249880 [details] [diff] [review] fix v1.0 Sexy. r=smorgan.
Attachment #249880 -
Flags: review?(stuart.morgan) → review+
Attachment #249880 -
Flags: superreview?(dbaron)
+ if ((SInt32)frameRect.size.width == smallScrollbarWidth || + (SInt32)frameRect.size.height == smallScrollbarWidth) { + [self setControlSize:NSSmallControlSize]; + } I want to add an else clause to that if statement that sets the scrollbar to normal size if neither dimension matches the small scrollbar width. That will fix the unlikely case in which we incorrectly make the native scrollbar small at some point before its final size is set. The reason we can get away with setting small size like this is that even if we have a large vertical scrollbar with a height of 16, that isn't a reasonable size so it doesn't matter if we incorrectly set the native scrollbar to small.
Comment 6•18 years ago
|
||
It is sexy :-) One problem I see with patch rv.1.0: The buttons on select widgets (dropdown) are squeezed to the width of the scrollbar. In Minefield, that makes for a odd looking button; in Camino, it results in the text overlapping the blue 'button'.
Comment 7•18 years ago
|
||
Camino (on top) and Minefield.
This fixes bug 311454, right?
Adds the fix I mentioned before and restricts the small scrollbars to listboxes, popup menus are not affected now.
Attachment #249880 -
Attachment is obsolete: true
Attachment #249933 -
Flags: superreview?(dbaron)
Attachment #249880 -
Flags: superreview?(dbaron)
Josh, with patch 1.1, it looks like select multiples lose their borders. Screenshot with today's 1.8branch nightly on top, a today's trunk+patch v1.1 in middle, and yesterday's trunk nightly on the bottom. Hyatt's form controls testcase: https://bugzilla.mozilla.org/attachment.cgi?id=99911&action=view
Comment 12•18 years ago
|
||
Comment on attachment 249933 [details] [diff] [review] fix v1.1 >+html|select[size], >+html|select[multiple], >+html|select[size][multiple] > scrollbar { Shouldn't this say: html|select[size] > scrollbar, html|select[multiple] > scrollbar { (<select multiple size=4> will match both rules of course) Do you need to make any changes to the @media print block?
Comment 13•18 years ago
|
||
1/
>+html|select[size],
>+html|select[multiple],
>+html|select[size][multiple] > scrollbar
surely you mean
html|select[size] > scrollbar,
html|select[multiple] > scrollbar,
html|select[size][multiple] > scrollbar {
What you select is: a select widget with the attribute [size], a select widget with the attribute [multiple] and a scrollbar that is the child of a select with the attributes [size] and [multiple].
That explains the display problems mentioned by Smokey.
2/
html|select[size] > scrollbar will also select some pop ups: <select size="0"> and <select size="1">.
html|select[size]:not([size="0"]):not([size="1"]) would work on trunk.
Not sure about Branch 1.8 (for porting to Camino 1.2 eventually). I remember having problems with that selector, an obscure parsing bug when there is a namespace declaration in the stylesheet.
The browser ending up reading the selector as
html|*select[size]:not([size="0"]):not([size="1"])
note the * before 'select'.
With both those changed on my side, that works fine on my Camino build here. Not yet tested with Minefield.
> Not sure about Branch 1.8 (for porting to Camino 1.2 eventually).
There's no hope of this landing on the 1.8 branch, unfortunately :( (Besides touching shared Core files, it depends on files that don't exist on the branch....)
Updated•18 years ago
|
Attachment #249951 -
Attachment is patch: false
Attachment #249951 -
Attachment mime type: text/plain → image/png
Comment 15•18 years ago
|
||
It seems that the text in the popup-style selects is also shifted up by 1px with the patch.
Assignee | ||
Comment 16•18 years ago
|
||
Comment on attachment 249933 [details] [diff] [review] fix v1.1 I'll post an updated patch soon.
Attachment #249933 -
Flags: superreview?(dbaron)
Assignee | ||
Comment 17•18 years ago
|
||
I used the extra CSS selector html|select[size][multiple] because that is done in layout/style/forms.css: 158 select[size], 159 select[multiple], 160 select[size][multiple] { If it is true that it is unnecessary, then we might want to fix that in forms.css as well.
Assignee | ||
Comment 18•18 years ago
|
||
Fixes stuff mentioned in comments. Thanks for all the help!
Attachment #249933 -
Attachment is obsolete: true
Attachment #250194 -
Flags: superreview?(dbaron)
Comment 19•18 years ago
|
||
(In reply to comment #17) >I used the extra CSS selector html|select[size][multiple] because that is done >in layout/style/forms.css: So, in that case the select[size][multiple] appears to be there to be more specific than the subsequent select[size="0"] and select[size="1"] rules. Your use of :not seems as if it should have the same effect.
Comment 20•18 years ago
|
||
(In reply to comment #18) > Created an attachment (id=250194) [details] > fix v1.1.1 > > Fixes stuff mentioned in comments. Thanks for all the help! > This works nicely on both Camino and Minefield. Not tested with SeaMonkey. (In reply to comment #19) >>I used the extra CSS selector html|select[size][multiple] because that is done >>in layout/style/forms.css: >So, in that case the select[size][multiple] appears to be there to be more >specific than the subsequent select[size="0"] and select[size="1"] rules. >Your use of :not seems as if it should have the same effect. I'm not sure about the history of that particular selector in forms.css. It is kind of redundant in that context, I think. And it selects different elements than the subsequent select[size="0"] and select[size="1"] selectors. The use of the html|select[size]:not([size="0"]):not([size="1"]) in this particular case is to prevent 'small scrollbars' to be applied to select[size="0"] and select[size="1"]. Anyway, the problem of forms.css belongs to another bug I think (and I'll run some tests to check if it is needed or not).
Attachment #250194 -
Flags: superreview?(dbaron) → superreview?(roc)
+ if ((SInt32)frameRect.size.width == smallScrollbarWidth || + (SInt32)frameRect.size.height == smallScrollbarWidth) { + [self setControlSize:NSSmallControlSize]; + } + else { + [self setControlSize:NSRegularControlSize]; + } Is this safe? What if we have a normal-sized scrollbar that is unusually short? Is it safe because a normal-sized scrollbar must be bigger than smallScrollbarWidth in both directions?
Assignee | ||
Comment 22•17 years ago
|
||
From comment #5 "The reason we can get away with setting small size like this is that even if we have a large vertical scrollbar with a height of 16, that isn't a reasonable size so it doesn't matter if we incorrectly set the native scrollbar to small."
Comment on attachment 250194 [details] [diff] [review] fix v1.1.1 ok I guess
Attachment #250194 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Comment 24•17 years ago
|
||
landed on trunk
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Filed bug 367571 as a followup.
You need to log in
before you can comment on or make changes to this bug.
Description
•