Closed
Bug 206636
Opened 21 years ago
Closed 16 years ago
Display default dialog buttons in pulsating blue color
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: phil.locke, Assigned: mstange)
References
(Blocks 1 open bug, )
Details
(Keywords: polish)
Attachments
(1 file, 1 obsolete file)
6.77 KB,
patch
|
jaas
:
review+
damons
:
approval1.9.1-
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.4b) Gecko/20030507 Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.4b) Gecko/20030507 i've noticed that many (or most, or all) of the dialogs do not have the default button highlighted. for example, in the dialog asking whether to save an unsent message in the drafts folder, the default button (activated by pressing Return) is "Save". in the dialog asking whether to send a message in plain text, text & html, or html only, the default is "Send". in the dialog asking whether to save the password for a particular page/site, the default is [i forget the exact name of the button, but it would be either "Save" or "Yes" or "OK"]. the highlight is an important visual cue to let users know whether they can perform the action using the keyboard or if they have to use the mouse. if there is a default, but it is not highlighted, the user won't know what will happen if they hit Return. Reproducible: Always Steps to Reproduce: 1. open a message and type some text, and close it -- that brings up one dialog or 2. open a message, type some text, address it to someone for whom mozilla doesn't know if they should be getting text or html or both, and hit send -- this brings up another dialog or 3. go to a site with a user name & password entry. put in your info & submit it. if you haven't already set your password preferences for the site, the other dialog i mentioned should appear. Actual Results: dialogs without highlighted defaults appeared Expected Results: presented dialogs with highlighted defaults
Well, bug 203734 is about showing the fuzzy focus ring around buttons, but I can't find a bug about displaying the default button in the pulsating blue color as in other OS X apps.
Assignee: sspitzer → jaggernaut
Blocks: 73812
Severity: minor → normal
Status: UNCONFIRMED → NEW
Component: Mail Window Front End → XP Toolkit/Widgets
Ever confirmed: true
Keywords: classic
Product: MailNews → Browser
QA Contact: esther → jrgm
Summary: dialogs do not have the default button appropriately highlighted → Display default dialog buttons in pulsating blue color
added trivial test URL: javascript:alert('foo'); note: same problem with Pinstripe theme (classic keyword currently set)
This bug is easily reproducible in Jaguar (MacOS 10.2), but seems to go away when running Firebird on Panther (MacOS 10.3).
Comment 5•18 years ago
|
||
*** Bug 326692 has been marked as a duplicate of this bug. ***
Comment 6•18 years ago
|
||
Changing component since this probably should be fixed in the widget code...
Assignee: jag → joshmoz
Component: XP Toolkit/Widgets → Widget: Mac
QA Contact: jrgmorrison → mac
Comment 7•18 years ago
|
||
*** Bug 236958 has been marked as a duplicate of this bug. ***
Updated•17 years ago
|
Severity: normal → minor
Component: Widget: Mac → Widget: Cocoa
Keywords: polish
QA Contact: mac → cocoa
Assignee | ||
Comment 8•16 years ago
|
||
There's no documented way of drawing default buttons with NSButtonCell, but we can do it with HITheme. This patch is on top of the patch in bug 468507.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Attachment #356555 -
Flags: superreview?(roc)
Attachment #356555 -
Flags: review?(joshmoz)
Assignee | ||
Updated•16 years ago
|
Attachment #356555 -
Flags: review?(enndeakin)
Assignee | ||
Comment 9•16 years ago
|
||
Comment on attachment 356555 [details] [diff] [review] fix v1 The "button-periodic-redraw" binding is very similar to the undetermined progressbar one: http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/progressmeter.xml?mark=117-150#117
Attachment #356555 -
Flags: superreview?(roc) → superreview+
Comment 10•16 years ago
|
||
Comment on attachment 356555 [details] [diff] [review] fix v1 >+ var width = hbox.boxObject.width; We should probably be avoiding using the boxObject and use getClientBoundingRect or something instead. No big deal though. >+button[default="true"] { >+ -moz-binding: url("chrome://global/content/bindings/button.xml#button-periodic-redraw"); I'm not familiar enough with css cascading, but does this work is someone had set <button type="menu" default="true"/> ? Not useful, but the menu binding should be used not the default=true one.
Attachment #356555 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 11•16 years ago
|
||
(In reply to comment #10) > >+button[default="true"] { > >+ -moz-binding: url("chrome://global/content/bindings/button.xml#button-periodic-redraw"); > > I'm not familiar enough with css cascading, but does this work is someone had > set <button type="menu" default="true"/> ? Not useful, but the menu binding > should be used not the default=true one. Hm, global.css is applied after xul.css, so right now the default=true binding overrides the menu binding. I can see three different ways to fix this: 1. Move the default=true binding to xul.css (ifdefed) above the other button bindings 2. Use button[default="true"]:not([type="menu"):not([type="repeat"])... as selector 3. Set !important on the other bindings in xul.css I prefer 1, what do you think?
Comment 12•16 years ago
|
||
1 is better yes, assuming its in a mac-only block.
Assignee | ||
Comment 13•16 years ago
|
||
Attachment #356555 -
Attachment is obsolete: true
Attachment #357328 -
Flags: review?(joshmoz)
Attachment #356555 -
Flags: review?(joshmoz)
Attachment #357328 -
Flags: review?(joshmoz) → review+
Assignee | ||
Comment 14•16 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/d1555870e786
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Hardware: PowerPC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Assignee | ||
Comment 15•16 years ago
|
||
When I checked in this patch, the mochichrome test for bug 398289 started to fail because it was comparing screenshots that also contained a default button. Since these snapshots were made at different times, the default button looked different in the snapshots. I fixed the failure by pushing the default button out of the snapshotted area. http://hg.mozilla.org/mozilla-central/rev/a17f34b6f31b
Comment 16•15 years ago
|
||
(In reply to comment #11) > Hm, global.css is applied after xul.css, so right now the default=true binding > overrides the menu binding. I can see three different ways to fix this: > 1. Move the default=true binding to xul.css (ifdefed) above the other button > bindings > 2. Use button[default="true"]:not([type="menu"):not([type="repeat"])... as > selector > 3. Set !important on the other bindings in xul.css > > I prefer 1, what do you think? But then 3rd party themes waste CPU cycles animating an inanimate button...
Assignee | ||
Updated•15 years ago
|
Attachment #357328 -
Flags: approval1.9.1?
Assignee | ||
Comment 17•15 years ago
|
||
Comment on attachment 357328 [details] [diff] [review] v2 nice polish, has baked for 2 months
Assignee | ||
Comment 18•15 years ago
|
||
(In reply to comment #16) > But then 3rd party themes waste CPU cycles animating an inanimate button... The CPU cycles will only be wasted on changing the step attribute, not on repainting it. The repaint is caused by nsNativeThemeCocoa::WidgetStateChanged which doesn't apply if the button isn't native-themed. But you're right, it would be nice if we didn't do that. Which of my suggestions in comment 11 do you prefer? Or do you have a different idea?
Comment 19•15 years ago
|
||
(In reply to comment #18) > (In reply to comment #16) > > But then 3rd party themes waste CPU cycles animating an inanimate button... > The CPU cycles will only be wasted on changing the step attribute, not on > repainting it. The repaint is caused by nsNativeThemeCocoa::WidgetStateChanged > which doesn't apply if the button isn't native-themed. OK, so although I don't know offhand what the penalty for updating an unused attribute is, I guess it can't be that bad. > But you're right, it would be nice if we didn't do that. Which of my > suggestions in comment 11 do you prefer? Or do you have a different idea? I'm tempted to suggest 2) but just using :not([type])
Comment 20•15 years ago
|
||
Comment on attachment 357328 [details] [diff] [review] v2 Need a test here. Re-request approval once we have this addressed.
Attachment #357328 -
Flags: approval1.9.1? → approval1.9.1-
You need to log in
before you can comment on or make changes to this bug.
Description
•