The default bug view has changed. See this FAQ.

Help button in Search Addresses dialog lacks Help icon

RESOLVED FIXED in seamonkey2.1a1

Status

SeaMonkey
MailNews: General
RESOLVED FIXED
8 years ago
2 years ago

People

(Reporter: Karsten Düsterloh, Assigned: Ian Neal)

Tracking

(Blocks: 1 bug)

Trunk
seamonkey2.1a1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 8 obsolete attachments)

(Reporter)

Description

8 years ago
The (custom) Help button in the Search Messages dialog lacks the Help icon used by standard dialog Help buttons. Furthermore, the Help button definition should come from helpMessengerOverlay.xul as well, so that ABSearchDialog.xul does not need to know about the OpenHelp function.
(Reporter)

Comment 1

8 years ago
Search Addresses dialog, of course.
Summary: Help button in Search Messages dialog lacks Help icon → Help button in Search Addresses dialog lacks Help icon
(Reporter)

Updated

8 years ago
OS: Linux → All
Hardware: x86 → All
(Assignee)

Comment 2

8 years ago
Created attachment 400043 [details] [diff] [review]
Change Search Messages/Addresses to dialog patch v0.1

This patch changes the SearchDialog.xul from being a <window> to being a <dialog> with standard help button.
Assignee: nobody → iann_bugzilla
Status: NEW → ASSIGNED
Attachment #400043 - Flags: ui-review?(stefanh)
Attachment #400043 - Flags: superreview?(neil)
Attachment #400043 - Flags: review?(mnyromyr)
(Assignee)

Comment 3

8 years ago
Created attachment 400045 [details]
Screenshot-Advanced Address Book Search
(Assignee)

Comment 4

8 years ago
Created attachment 400046 [details]
Screenshot-Search Messages

Comment 5

8 years ago
Comment on attachment 400043 [details] [diff] [review]
Change Search Messages/Addresses to dialog patch v0.1

As per bug 515227. Oh, and if you really want the help button to be in the overlay, then give the container an id and overlay the container to add it.
Attachment #400043 - Flags: superreview?(neil) → superreview-

Comment 6

8 years ago
You should put the help button in the lower right corner of the dialog instead.
(Assignee)

Updated

8 years ago
Attachment #400043 - Flags: ui-review?(stefanh)
Attachment #400043 - Flags: review?(mnyromyr)
(Assignee)

Comment 7

8 years ago
Created attachment 400267 [details] [diff] [review]
Change Search Messages/Addresses to dialog without overlay patch v0.1a

Changes since v0.1:
* searchDialog.css now imports global dialog.css for default theme.
* help button is now moved to where help button is usually found on a dialog
(dlgtype="help" seems to give icon="help" automagically).
* added class="dialog-button" to help button.
Attachment #400043 - Attachment is obsolete: true
Attachment #400267 - Flags: ui-review?(stefanh)
Attachment #400267 - Flags: superreview?(neil)
Attachment #400267 - Flags: review?(mnyromyr)
(Assignee)

Comment 8

8 years ago
Created attachment 400270 [details]
Screenshot-Advanced Address Book Search (moved Help button)
Attachment #400045 - Attachment is obsolete: true
(Assignee)

Comment 9

8 years ago
Created attachment 400271 [details]
Screenshot-Search Messages (moved Help button)
Attachment #400046 - Attachment is obsolete: true

Comment 10

8 years ago
Is it possible to put the Help button in the right corner?
(Assignee)

Comment 11

8 years ago
(In reply to comment #10)
> Is it possible to put the Help button in the right corner?

Yes, but would have to put some #ifdef's in for *nix vs the rest.
(In reply to comment #0)
> the Help button definition should come from helpMessengerOverlay.xul as well,
> so that ABSearchDialog.xul does not need to know about the OpenHelp function.
helpMessengerOverlay.xul was only created to add buttons to shared dialogs...

(In reply to comment #10)
> Is it possible to put the Help button in the right corner?
It would certainly make more sense. It looks terrible in the left corner.
Comment on attachment 400267 [details] [diff] [review]
Change Search Messages/Addresses to dialog without overlay patch v0.1a

>-<window id="searchAddressBookWindow"
>+<dialog id="searchAddressBookWindow"
I still don't see the point of the window/dialog conversion.
(Reporter)

Comment 14

8 years ago
Comment on attachment 400267 [details] [diff] [review]
Change Search Messages/Addresses to dialog without overlay patch v0.1a

What's the reason for The Great Accesskey Renaming?

Basically, the lone Help button down left is rather pointless. Just move one line up and to the outer right there.

Furthermore, the top address book dropdown should flex horizontally.
Attachment #400267 - Flags: review?(mnyromyr) → review-
(Assignee)

Updated

8 years ago
Attachment #400267 - Flags: ui-review?(stefanh)
Attachment #400267 - Flags: superreview?(neil)
(Assignee)

Comment 15

8 years ago
Created attachment 401966 [details] [diff] [review]
Change Search Messages/Addresses to dialog help on right patch v0.1b

Changes since v0.1b:
* Moved to right and inline with rest of the buttons in both dialogs.

Help now takes H accesskey (as is the default on most other dialogs/windows) hence the accesskey shuffle.
Attachment #400267 - Attachment is obsolete: true
Attachment #401966 - Flags: review?(mnyromyr)
(Assignee)

Comment 16

8 years ago
Created attachment 401967 [details]
Screenshot-Advanced Address Book Search right help
Attachment #400270 - Attachment is obsolete: true
(Assignee)

Comment 17

8 years ago
Created attachment 401968 [details]
Screenshot-Search Messages right help
Attachment #400271 - Attachment is obsolete: true
(Reporter)

Comment 18

8 years ago
Comment on attachment 401966 [details] [diff] [review]
Change Search Messages/Addresses to dialog help on right patch v0.1b

Basically okay, just some nits:

>+++ b/suite/locales/en-US/chrome/mailnews/SearchDialog.dtd
> <!ENTITY openButton.label            "Open">
> <!ENTITY openButton.accesskey        "n">

I think this Open should get the O...

>+<dialog id="searchAddressBookWindow"
>         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>         xmlns:nc="http://home.netscape.com/NC-rdf#"
>         onload="searchOnLoad();"
>         onunload="onSearchStop(); searchOnUnload();"
>+        buttons="help"
>+        ondialoghelp="return openHelp('mail_advanced_ab_search');"

Command handler attributes should go last.

>           <label value="&abSearchHeading.label;" accesskey="&abSearchHeading.accesskey;" control="abPopup"/>
>           <menulist id="abPopup" oncommand="SelectDirectory(this.value);">
>             <menupopup id="abPopup-menupopup" class="addrbooksPopup"/>
>           </menulist>
>           <spacer flex="10"/>
>           <button id="search-button" oncommand="onSearchButton(event);" default="true"/>

Linux Classic at least has a problem with styling menulists. Although there should be enough space here, the menuitem "Personal Address Book" gets truncated to "Personal Addre..." in the menulist and to "Personal Address..." in the popup. How'd you feel about making the top menulist in all three dialogs (Message Search, AB Search, Filters) flex?

>+        <button dlgtype="help" class="dialog-button"/>

As with the filters dialog, the Help button draws an unwanted "margin-top: 6px" rule here. :-/

>+<dialog id="searchMailWindow"
>         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
>         xmlns:nc="http://home.netscape.com/NC-rdf#"
>         onload="searchOnLoad();"
>         onunload="onSearchStop(); searchOnUnload();"
>+        buttons="help"
>+        ondialoghelp="return openHelp('search_messages');"

Command handler attributes should go last.
Attachment #401966 - Flags: review?(mnyromyr) → review-
(Assignee)

Comment 19

8 years ago
Created attachment 403035 [details] [diff] [review]
Change Search Messages/Addresses to dialog with command last patch v0.1c

Changes since v0.1b:
* Removed 6px margin from the top of help button in classic.
* Changed order of attributes on dialog box.

As discussed changes to size of menuitem will happen in bug 515967.
Attachment #401966 - Attachment is obsolete: true
Attachment #403035 - Flags: review?(mnyromyr)
(Reporter)

Updated

8 years ago
Attachment #403035 - Flags: review?(mnyromyr) → review+
(Assignee)

Updated

8 years ago
Attachment #403035 - Flags: superreview?(neil)
Testing this patch made me notice that there's a bug in searchDialog.css (both themes) whereby for some reason the padding on menuitems is overridden. This unfortunately isn't the whole story for address book search, because it suffers from the additional indignity that the width of menulists is overridden, when in fact only the message folders menulist should have its width set.

When I added the "Search local system" checkbox to SearchDialog.xul it was convenient that I could add it to what was a blank cell in the "grid". I'm not sure that I would have put it there had the Help button been at the bottom.

Now that these are dialogs, should we remove the Ctrl+W key to close them?
(Assignee)

Comment 21

8 years ago
(In reply to comment #20)
> Testing this patch made me notice that there's a bug in searchDialog.css (both
> themes) whereby for some reason the padding on menuitems is overridden. This
> unfortunately isn't the whole story for address book search, because it suffers
> from the additional indignity that the width of menulists is overridden, when
> in fact only the message folders menulist should have its width set.
See my comment#19
> 
> When I added the "Search local system" checkbox to SearchDialog.xul it was
> convenient that I could add it to what was a blank cell in the "grid". I'm not
> sure that I would have put it there had the Help button been at the bottom.
So should it go in parallel to the other checkbox?
> 
> Now that these are dialogs, should we remove the Ctrl+W key to close them?
Yes, probably.
(In reply to comment #21)
> See my comment#19
Sorry, I didn't realise the significance.

> So should it go in parallel to the other checkbox?
Yes, probably ;-)
(Assignee)

Updated

8 years ago
Attachment #403035 - Flags: superreview?(neil)
(Assignee)

Comment 23

8 years ago
Created attachment 405366 [details] [diff] [review]
Remove Close Window key patch v0.1d [Checkin: comment 27]

Changes since v0.1c:
* Move checkboxes in Search Messages to be in parallel.
* Remove Ctrl-W from new dialog boxes.
Attachment #403035 - Attachment is obsolete: true
Attachment #405366 - Flags: superreview?(neil)
Attachment #405366 - Flags: review+

Comment 24

8 years ago
Regarding the removal of Accel+W: Is this just because it's a dialog? If that's the case, is there anyone that will think it's a dialog? I mean, I have never seen a dialog with a status bar...

Updated

8 years ago
Attachment #405366 - Flags: superreview?(neil) → superreview+
(Assignee)

Comment 25

8 years ago
Comment on attachment 405366 [details] [diff] [review]
Remove Close Window key patch v0.1d [Checkin: comment 27]

I'll ask for a= as even there are locale changes they are just accesskeys and the one string that is removed, even if localizers don't remove it it should make no difference.
Attachment #405366 - Flags: approval-seamonkey2.0?

Comment 26

8 years ago
Comment on attachment 405366 [details] [diff] [review]
Remove Close Window key patch v0.1d [Checkin: comment 27]

I don't like the locale changes for 2.0 final, esp. not after RC1 has been cut. The accesskeys would possibly be OK, but the removed strings mean all dashboard entries turning orange, which is not something I'd like to have in final release preparations.
We can probably look into taking that for 2.0.1 with the necessary thunder in the L10n group/list, but please not before 2.0 proper.
Attachment #405366 - Flags: approval-seamonkey2.0? → approval-seamonkey2.0-
(Assignee)

Comment 27

8 years ago
Comment on attachment 405366 [details] [diff] [review]
Remove Close Window key patch v0.1d [Checkin: comment 27]

http://hg.mozilla.org/comm-central/rev/cefd439cd3c2
Attachment #405366 - Attachment description: Remove Close Window key patch v0.1d → Remove Close Window key patch v0.1d [Checkin: comment 27]
(Assignee)

Updated

8 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1a1
The switch to a dialog made here caused bug 543160.

Updated

7 years ago
Depends on: 543160
Blocks: 672155
(In reply to comment #24)
> Regarding the removal of Accel+W: Is this just because it's a dialog? If
> that's the case, is there anyone that will think it's a dialog? I mean, I
> have never seen a dialog with a status bar...

I don't really get the logic behind that either; the whole reasoning that was put into words here comes down to "because it's a dialog [<dialog>], Ctrl+W should not close it", but I don't understand where that's coming from. Please enlighten us and leave your comments over at bug 672155.

Comment 30

2 years ago
Are we talking about the lifebelt symbol in Help button left from "Help" text?
I still do not see that  with EN-US Seamonkey 2.33.1 (German Language pack)  Gecko/20100101 Build 20150321194901 (Default Theme) on German WIN7 64bit.
Flags: needinfo?(iann_bugzilla)

Comment 31

2 years ago
Hm, I never saw that lifebelt in any help button.
(Assignee)

Comment 32

2 years ago
I remember it in the past, but I think it depends on the OS (and OS theme).
Flags: needinfo?(iann_bugzilla)

Comment 33

2 years ago
Yeah, it's an OS icon I think, and as such depends on the desktop theme.
See attachment 8602370 [details] for a prefpane example on Linux with KDE4.
You need to log in before you can comment on or make changes to this bug.