Closed Bug 610517 Opened 14 years ago Closed 14 years ago

Bookmarks and Address Book sidebars' search fields miss placeholder attribute

Categories

(SeaMonkey :: Sidebar, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.1b2

People

(Reporter: InvisibleSmiley, Assigned: InvisibleSmiley)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch patch (obsolete) — Splinter Review
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
Attachment #489010 - Flags: superreview?(neil)
Attachment #489010 - Flags: review?(mnyromyr)
I'm not convinced that the address book sidebar panel needs placeholder text because the search field is already part of a phrase.
(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 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.
Attachment #489010 - Flags: superreview?(neil) → superreview-
Attached patch patch v2 (obsolete) — Splinter Review
Attachment #489010 - Attachment is obsolete: true
Attachment #489325 - Flags: superreview?(neil)
Attachment #489325 - Flags: review?(mnyromyr)
Attachment #489010 - Flags: review?(mnyromyr)
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.
Attachment #489325 - Flags: superreview?(neil) → superreview+
(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.
Attachment #489325 - Attachment is obsolete: true
Attachment #489517 - Flags: superreview+
Attachment #489517 - Flags: review?
Attachment #489325 - Flags: review?(mnyromyr)
Attachment #489517 - Flags: review? → review+
Comment on attachment 489517 [details] [diff] [review]
patch v2a [Checkin: comment 7]

http://hg.mozilla.org/comm-central/rev/7865178cde4a
Attachment #489517 - Attachment description: patch v2a → patch v2a [Checkin: comment 7]
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b2
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: