Last Comment Bug 610517 - Bookmarks and Address Book sidebars' search fields miss placeholder attribute
: Bookmarks and Address Book sidebars' search fields miss placeholder attribute
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Sidebar (show other bugs)
: Trunk
: All All
: -- minor (vote)
: seamonkey2.1b2
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-08 16:02 PST by Jens Hatlak (:InvisibleSmiley)
Modified: 2010-11-10 12:07 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (4.17 KB, patch)
2010-11-08 16:02 PST, Jens Hatlak (:InvisibleSmiley)
neil: superreview-
Details | Diff | Splinter Review
patch v2 (5.39 KB, patch)
2010-11-09 15:46 PST, Jens Hatlak (:InvisibleSmiley)
neil: superreview+
Details | Diff | Splinter Review
patch v2a [Checkin: comment 7] (5.39 KB, patch)
2010-11-10 09:03 PST, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review+
jh: superreview+
Details | Diff | Splinter Review

Description Jens Hatlak (:InvisibleSmiley) 2010-11-08 16:02:17 PST
Created attachment 489010 [details] [diff] [review]
patch

We have four default sidebars. All of them contain quick search fields, but only two of those show a description when not focused (which is achieved using the placeholder attribute on the textbox elements).

OK:
suite/common/search/search-panel.xul
suite/common/history/history-panel.xul

To do:
mailnews/addrbook/content/addressbook-panel.xul
suite/common/bookmarks/bm-panel.xul
Comment 1 neil@parkwaycc.co.uk 2010-11-09 04:28:48 PST
I'm not convinced that the address book sidebar panel needs placeholder text because the search field is already part of a phrase.
Comment 2 Jens Hatlak (:InvisibleSmiley) 2010-11-09 05:07:17 PST
(In reply to comment #1)
> I'm not convinced that the address book sidebar panel needs placeholder text
> because the search field is already part of a phrase.

Or we could leave out the "Search " prefix, i.e. make it "for: [Name or Email    ]".
Comment 3 neil@parkwaycc.co.uk 2010-11-09 05:41:29 PST
Comment on attachment 489010 [details] [diff] [review]
patch

OK, that would work, but you'd want to change the select addresses dialog too.

As for bookmarks, if you're using placeholder then you can lose the label.
Comment 4 Jens Hatlak (:InvisibleSmiley) 2010-11-09 15:46:25 PST
Created attachment 489325 [details] [diff] [review]
patch v2
Comment 5 neil@parkwaycc.co.uk 2010-11-10 03:41:38 PST
Comment on attachment 489325 [details] [diff] [review]
patch v2

>--- a/mailnews/addrbook/content/abSelectAddressesDialog.xul
>+++ b/mailnews/addrbook/content/abSelectAddressesDialog.xul
[Odd, why are these files not in suite/mailnews/addrbook?]

>+<!ENTITY for.placeholder                     "Name and Email"> 
Shouldn't this be "Name or Email"? sr=me with that fixed.
Nit: trailing space.
Comment 6 Jens Hatlak (:InvisibleSmiley) 2010-11-10 09:03:46 PST
Created attachment 489517 [details] [diff] [review]
patch v2a [Checkin: comment 7]

(In reply to comment #5)
> >+++ b/mailnews/addrbook/content/abSelectAddressesDialog.xul
> [Odd, why are these files not in suite/mailnews/addrbook?]

I guess because no-one cared to file and fix a bug for it yet, even though some noticed? Cf. bug 507622 comment 0. Filed bug 611029.

> >+<!ENTITY for.placeholder                     "Name and Email"> 
> Shouldn't this be "Name or Email"? sr=me with that fixed.

I wanted to achieve consistency with the placeholder of the Address Book main window's search field but missed the difference in context.
Comment 7 Jens Hatlak (:InvisibleSmiley) 2010-11-10 12:06:48 PST
Comment on attachment 489517 [details] [diff] [review]
patch v2a [Checkin: comment 7]

http://hg.mozilla.org/comm-central/rev/7865178cde4a

Note You need to log in before you can comment on or make changes to this bug.