Closed
Bug 495939
Opened 15 years ago
Closed 14 years ago
New Folder dialog don't use a verb for the confirm button
Categories
(Thunderbird :: Mail Window Front End, defect)
Thunderbird
Mail Window Front End
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.1b2
People
(Reporter: andreasn, Assigned: ewong)
Details
Attachments
(2 files, 3 obsolete files)
16.69 KB,
image/png
|
Details | |
2.64 KB,
patch
|
standard8
:
review+
neil
:
superreview+
andreasn
:
ui-review+
|
Details | Diff | Splinter Review |
According to the Interface guidelines for both Vista, OS X and GNOME, it's preferred to use a verb instead of a standard button labeled OK. I would suggest "Create" or "Create folder"
Comment 1•15 years ago
|
||
yeah, I'd choose "Create folder"
Comment 2•15 years ago
|
||
With a capital F please
Updated•15 years ago
|
Severity: normal → minor
Component: General → Mail Window Front End
QA Contact: general → front-end
Whiteboard: [good first bug]
Assignee | ||
Comment 3•14 years ago
|
||
- Changed the default "OK" button to 'Create Folder' as per the description and comments.
Assignee | ||
Comment 4•14 years ago
|
||
The original patch was incorrect. It should be a modification of both the suite/locales/en-US/chrome/mailnews/newFolderDialog.dtd and mail/locales/en_US/chrome/messenger/newFolderDialog.dtd instead.
Attachment #428375 -
Attachment is obsolete: true
Reporter | ||
Comment 5•14 years ago
|
||
Woohoo! Sweet! Let me know if you need a ui-review.
Comment 6•14 years ago
|
||
(In reply to comment #4) > Created an attachment (id=428386) [details] > Replacing previous patch after review by philor > > The original patch was incorrect. It should be a modification of both > the suite/locales/en-US/chrome/mailnews/newFolderDialog.dtd and > mail/locales/en_US/chrome/messenger/newFolderDialog.dtd instead. To get the patch in Edmund you need to follow https://developer.mozilla.org/en/comm-central#Requirements and as your patch touches UI you'll need a ui review (hint andreas volunteered)!
Assignee | ||
Updated•14 years ago
|
Attachment #428386 -
Flags: review?(nisses.mail)
Assignee | ||
Updated•14 years ago
|
Attachment #428386 -
Flags: review?(nisses.mail) → ui-review?(nisses.mail)
Reporter | ||
Updated•14 years ago
|
OS: Linux → All
Reporter | ||
Comment 7•14 years ago
|
||
I think there might be pieces missing from attachment 428386 [details] [diff] [review] that was in the first patch as I can't get this to work on two of my systems. We can look into it tomorrow on IRC.
Assignee | ||
Comment 8•14 years ago
|
||
(In reply to comment #7) > I think there might be pieces missing from attachment 428386 [details] [diff] [review] that was in the > first patch as I can't get this to work on two of my systems. We can look into > it tomorrow on IRC. Strange. Is it because of the no newline at the end of the 2nd part of the diff?
Assignee | ||
Comment 9•14 years ago
|
||
- Fixed the no newline. - Added the appropriate tags in the newFolderDialog.xul as suggested by Standard8.
Attachment #428386 -
Attachment is obsolete: true
Attachment #428870 -
Flags: review?(nisses.mail)
Attachment #428386 -
Flags: ui-review?(nisses.mail)
Comment 10•14 years ago
|
||
(In reply to comment #9) > - Added the appropriate tags in the newFolderDialog.xul as suggested by > Standard8. I think you forgot to add accept.accesskey to the dtd files...
Assignee | ||
Comment 11•14 years ago
|
||
Attachment #428870 -
Attachment is obsolete: true
Attachment #428893 -
Flags: review?(nisses.mail)
Attachment #428870 -
Flags: review?(nisses.mail)
Reporter | ||
Comment 12•14 years ago
|
||
Sweet, got it to work on one of my machines now! You probably want to set me as ui-r, rather than r. Who is best to set as (code) reviewer? Magnus?
Reporter | ||
Comment 13•14 years ago
|
||
Comment on attachment 428893 [details] [diff] [review] Updated previous patch to include accesskey tags in the dtd files [Checkin: Comment 18] Setting myself as ui-reviewer, as I assume that was the intent. (I make mistakes like that all the time as well :)
Attachment #428893 -
Flags: review?(nisses.mail) → ui-review?(nisses.mail)
Assignee | ||
Comment 14•14 years ago
|
||
(In reply to comment #13) > (From update of attachment 428893 [details] [diff] [review]) > Setting myself as ui-reviewer, as I assume that was the intent. (I make > mistakes like that all the time as well :) Yes. My bad. It was supposed to be ui-reviewer. c-c isn't compiling on my box due to some manifest issue.
Assignee | ||
Updated•14 years ago
|
Status: NEW → ASSIGNED
Reporter | ||
Comment 15•14 years ago
|
||
Comment on attachment 428893 [details] [diff] [review] Updated previous patch to include accesskey tags in the dtd files [Checkin: Comment 18] Looks good to me, now we need a code review from someone as well.
Attachment #428893 -
Flags: ui-review?(nisses.mail) → ui-review+
Reporter | ||
Comment 16•14 years ago
|
||
Comment on attachment 428893 [details] [diff] [review] Updated previous patch to include accesskey tags in the dtd files [Checkin: Comment 18] Setting Mark as code reviewer and neil as super reviewer as per Mark's suggestion on irc.
Attachment #428893 -
Flags: superreview?(neil)
Attachment #428893 -
Flags: review?(bugzilla)
Updated•14 years ago
|
Attachment #428893 -
Flags: superreview?(neil) → superreview+
Comment 17•14 years ago
|
||
Comment on attachment 428893 [details] [diff] [review] Updated previous patch to include accesskey tags in the dtd files [Checkin: Comment 18] Looks good, thanks for the patch.
Attachment #428893 -
Flags: review?(bugzilla) → review+
Updated•14 years ago
|
Keywords: checkin-needed
Comment 18•14 years ago
|
||
Comment on attachment 428893 [details] [diff] [review] Updated previous patch to include accesskey tags in the dtd files [Checkin: Comment 18] http://hg.mozilla.org/comm-central/rev/a55720d38ec1
Attachment #428893 -
Attachment description: Updated previous patch to include accesskey tags in the dtd files. → Updated previous patch to include accesskey tags in the dtd files
[Checkin: Comment 18]
Updated•14 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Hardware: x86 → All
Resolution: --- → FIXED
Whiteboard: [good first bug]
Target Milestone: --- → Thunderbird 3.1b2
You need to log in
before you can comment on or make changes to this bug.
Description
•