The "RSS subscription" dialog should have an "OK" button

RESOLVED FIXED in Thunderbird 29.0

Status

--
enhancement
RESOLVED FIXED
12 years ago
4 years ago

People

(Reporter: alfred.peng, Assigned: alta88)

Tracking

Trunk
Thunderbird 29.0
x86
All

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
Thunderbird version 3 alpha 1 (20070115) on OpenSolaris.

Bring up Account Settings, select the RSS account, then click the Manage Subscriptions button.

Actual result:
The users have to click right-up conner close button to close.

Expected result:
There should have an "OK" button.
I think this is is intended. See e.g. bug 183419 comment 34. 
Assignee: mscott → nobody

Comment 2

10 years ago
UI man.
This sounds like a windows vs. mac thing.  I believe the Mac HIG recommends against a close button for dialogs, in favour of the button in the title bar.  However windows and linux/gnome recommend explicit close/ok buttons for most dialogs.

Updated

10 years ago
Component: RSS → Feed Reader
Product: Thunderbird → MailNews Core
(Assignee)

Updated

5 years ago
Duplicate of this bug: 529457
(Assignee)

Comment 5

5 years ago
This bug should either be closed or fixed. I lean toward adding a Close button, on the same line as the other 5, with a flex between them and Close.
Flags: needinfo?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(josiah)
Flags: needinfo?(bwinton)
Yes, we should add a close button.

Windows and OS X definitely require a close button, separate from the default window controls. In fact, even OS X does in this case.

As seen from: https://developer.apple.com/library/mac/documentation/userexperience/conceptual/applehiguidelines/Windows/Windows.html

Only "panels" (popup windows that float on top) and document windows, do not require a close button. Dialogs and Alerts on the other hand do. Technically this window is a document window as the Cocoa APIs define it, though we use it more like a dialog ourselves.

Therefore I see no reason to special case OS X. Let's just add a "Done" button to the right of the existing buttons.
Flags: needinfo?(josiah)
I'm also okay with a close button, although we have already a lot of buttons on one line in this small (in default) dialog.
(Assignee)

Comment 8

5 years ago
Created attachment 8367958 [details] [diff] [review]
closebutton.patch


It's not ideal given the existing button row, but a second row would be worse.  The text is 'Close', as this implies neither update/apply (like Done or Ok) nor undo/cancel.

The newsgroup subscribe dialog has Ok and Cancel in both win and linux.  The options/preference dialog has Ok and Cancel on win and Close on linux, fwiw.  Seems Ok and Cancel aren't right on instant apply dialogs.
Assignee: nobody → alta88
Attachment #8367958 - Flags: review?(josiah)
(I think you've got the info you needed, Alta.  ;)
Flags: needinfo?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(bwinton)
Comment on attachment 8367958 [details] [diff] [review]
closebutton.patch

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

Looks fine to me. Thanks!
Attachment #8367958 - Flags: review?(josiah) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/704cbe92153e
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0

Comment 12

5 years ago
File Not Found

The file chrome://messenger/locale/preferences/permissions.dtd cannot be found. Please check the location and try again.&f=regular

    Could the item have been renamed, removed, or relocated?
    Is there a spelling, capitalization, or other typographical error in the address?
    Do you have sufficient access permissions to the requested item?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 13

5 years ago
1. You shouldn't re-purpose strings from some other random .DTD since the semantics might be different in languages that aren't English.

2. Instead add an entity to feed-subscriptions.dtd (Both Thunderbird and SeaMonkey please). Your assistance in this matter is greatly appreciated.
(Assignee)

Comment 14

5 years ago
Created attachment 8369489 [details] [diff] [review]
closebutton2.patch
Attachment #8369489 - Flags: review+
(Assignee)

Comment 15

5 years ago
(In reply to Philip Chee from comment #13)
> 1. You shouldn't re-purpose strings from some other random .DTD since the
> semantics might be different in languages that aren't English.
> 

True, just not in this case.

> 2. Instead add an entity to feed-subscriptions.dtd (Both Thunderbird and
> SeaMonkey please). Your assistance in this matter is greatly appreciated.

Yes, duplication can't be avoided unfortunately.  Sorry for the oversight.
Keywords: checkin-needed
(Assignee)

Comment 16

5 years ago
Created attachment 8369494 [details] [diff] [review]
closebutton2.patch
Attachment #8369489 - Attachment is obsolete: true
Attachment #8369494 - Flags: review+

Comment 17

5 years ago
Comment on attachment 8369494 [details] [diff] [review]
closebutton2.patch

rs=me for SeaMonkey
Attachment #8369494 - Flags: review+
https://hg.mozilla.org/comm-central/rev/5c89bc7e77e9
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED

Updated

5 years ago
Duplicate of this bug: 975324

Updated

5 years ago
Duplicate of this bug: 1006056

Updated

5 years ago
Duplicate of this bug: 1007815

Comment 22

5 years ago
Comment on attachment 8369494 [details] [diff] [review]
closebutton2.patch

This follow-up patch made it for the 30.0 release cycle (currently comm-beta) but barely missed the 29.0 cut-off, hence quite a few duplicates have been accumulated for SM 2.26. If this should go into a 2.26.1 release it'll need to land on comm-release as well.
Attachment #8369494 - Flags: approval-comm-release?
Flags: needinfo?(bugspam.Callek)

Comment 23

5 years ago
Comment on attachment 8369494 [details] [diff] [review]
closebutton2.patch

>+++ b/suite/locales/en-US/chrome/mailnews/newsblog/feed-subscriptions.dtd
>+<!ENTITY button.close.label          "Close">

Eh, this has a string change (thanks Ratty!), thus cancelling requests. :-(
Attachment #8369494 - Flags: approval-comm-release?

Updated

5 years ago
Flags: needinfo?(bugspam.Callek)

Updated

5 years ago
Duplicate of this bug: 1028089

Updated

5 years ago
Duplicate of this bug: 1028821

Updated

4 years ago
Duplicate of this bug: 1044557

Updated

4 years ago
Duplicate of this bug: 1048060
You need to log in before you can comment on or make changes to this bug.