The default bug view has changed. See this FAQ.

Implement <input type="search">

RESOLVED FIXED in mozilla1.9.3a5

Status

()

Core
Layout: Form Controls
--
enhancement
RESOLVED FIXED
9 years ago
3 years ago

People

(Reporter: Andy Lyttle, Assigned: mounir)

Tracking

(Blocks: 4 bugs, {dev-doc-complete, html5})

Trunk
mozilla1.9.3a5
dev-doc-complete, html5
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [parity-webkit], URL)

Attachments

(3 attachments, 7 obsolete attachments)

(Reporter)

Description

9 years ago
We should implement <input type="search"> in a Safari-compatible way.  Attributes are:

placeholder="Search" - display this text in gray, when the value is empty and the field does not have focus

onsearch="foo();" - JavaScript function called when the user initiates a search

incremental - if present, onsearch fires on every keypress, and on clicking the cancel button; otherwise onsearch fires when pressing Enter

results - if present, shows a little magnifying glass icon, which helps to visually identify the field as a search box

results=10 - the magnifying glass functions as a drop-down menu containing a history of the last (whatever number) search terms

autosave="foo" - define a unique name to identify the search history (so the same history may be shared across multiple search boxes, even across different domains)

All other attributes of <input type="text"> are also supported.

Apple's implementation adds a cancel (x) button which appears whenever text has been typed into the box; clicking the cancel button clears the text without removing focus.  They also use rounded corners to make search boxes visually distinct from plain text boxes.  Pictures:
http://alexking.org/blog/2006/11/12/safari-search-boxes

Google has indicated that they are interested in adding support in Chrome:
http://code.google.com/p/chromium/issues/detail?id=17#c9

A relevant comment:
http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2007-May/011367.html
(Reporter)

Updated

9 years ago
Depends on: 457800

Updated

8 years ago
Component: General → Layout: Form Controls
Product: Firefox → Core
QA Contact: general → layout.form-controls
Keywords: helpwanted
(Assignee)

Comment 1

7 years ago
Actually, in HTML 5 specifications, the search input should behave exactly like a text input [1]. The results, onsearch and autosave attribute have been added by webkit.
The search input aims to improve accessibility by telling the user this input will be used for a search. We can tweak the style of the element to make sure it is clear it is here to search but you have to be cautious because it should not be used by webdevs for the "cool" looking [2].

By the way, webkit adds a "cancel" button which clear the value of the input.

[1] http://www.w3.org/TR/html5/forms.html#text-state-and-search-state
[2] http://lists.whatwg.org/htdig.cgi/whatwg-whatwg.org/2007-May/011367.html
(Assignee)

Updated

7 years ago
No longer depends on: 457800
(Assignee)

Updated

7 years ago

Updated

7 years ago
Keywords: html5
Whiteboard: [parity-webkit]
Version: unspecified → Trunk
(Assignee)

Updated

7 years ago
Assignee: nobody → mounir.lamouri
Keywords: helpwanted → dev-doc-needed
Summary: Implement <input type="search"> for compatibility with Safari → Implement <input type="search">
(Assignee)

Comment 2

7 years ago
Created attachment 435209 [details] [diff] [review]
Tests v0.1
Attachment #435209 - Flags: review?(jonas)
(Assignee)

Comment 3

7 years ago
Created attachment 435211 [details] [diff] [review]
Patch v0.1
Attachment #435211 - Flags: review?(jonas)
(Assignee)

Comment 4

7 years ago
As I said in comment 1, the search state is exactly the same as the text state so it's somewhat weird to implement. I've added NS_FORM_INPUT_SEARCH when I think it was needed and the tests are minimalistic. I don't think it is doable to test everything linked to the search state as I will have to test everything linked to the text state...

I think the interesting part would be the per-platform style. I don't know if the cancel button inside the search box is really needed. Maybe on MacOS X to keep the general UI style ?
Status: NEW → ASSIGNED
(Assignee)

Comment 5

7 years ago
CCing:
- David for accessibility. This new input state may interest you.
- Alex for UI questions.
(Reporter)

Comment 6

7 years ago
Created attachment 435353 [details]
Sample search field from Windows 7

A search field from Windows 7, which includes an "x" cancel button.
check out the xul searchbox element to see how to style a search box for each platform (or perhaps directly use this element if possible, not entirely up to speed on what HTML5 defines for a search box).
(Assignee)

Comment 8

7 years ago
Created attachment 436663 [details]
Search Box in GTK

The same style could be used for the input search state. As far as I know, depending on the platform, the magnifying glass has not the same look.
The magnifying glass will block the text (as it is for the search box) but will not be clickable (as opposite as the search box).

Alex, will you agree with this style ?
(Assignee)

Comment 9

7 years ago
By the way, the specs does say we can style the search input so I think we can do the styling in a follow if you agree Jonas.
I would prefer we did the styling in a follow up bug.
Attachment #435209 - Flags: review?(jonas) → review+
Attachment #435211 - Flags: review?(jonas) → review+
(Assignee)

Updated

7 years ago
Attachment #435209 - Flags: superreview?(Olli.Pettay)
(Assignee)

Comment 11

7 years ago
Comment on attachment 435211 [details] [diff] [review]
Patch v0.1

Thank you for your review Jonas.
Olli, as you will review the telephone patch, could you sr this one ? They are very similar.
Attachment #435211 - Flags: superreview?(Olli.Pettay)
(Assignee)

Comment 12

7 years ago
Created attachment 437878 [details] [diff] [review]
Tests v2

I've added new tests based on comments I got from Olli in bug 557620.
Attachment #435209 - Attachment is obsolete: true
Attachment #435209 - Flags: superreview?(Olli.Pettay)
(Assignee)

Comment 13

7 years ago
Created attachment 437880 [details] [diff] [review]
Patch v2

This patch is based on patch from bug 557620 which is introducing helper methods.
Attachment #435211 - Attachment is obsolete: true
Attachment #435211 - Flags: superreview?(Olli.Pettay)
(Assignee)

Comment 14

7 years ago
Adding bug 557620 and bug 551670 in depend list because attached patches are based on patches from those bugs.

Jonas, do you want to re-review ?
Depends on: 557620, 551670
(Assignee)

Updated

7 years ago
Blocks: 558594
(Assignee)

Updated

7 years ago
Attachment #436663 - Attachment is obsolete: true
(Assignee)

Comment 15

7 years ago
Comment on attachment 435353 [details]
Sample search field from Windows 7

Let's talk about the field style in bug 558594.
Attachment #435353 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Blocks: 558651
(Assignee)

Comment 16

7 years ago
Comment on attachment 437878 [details] [diff] [review]
Tests v2

A few tests have been added. It's very similar to the tests from bug 557620.
Attachment #437878 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 17

7 years ago
Comment on attachment 437880 [details] [diff] [review]
Patch v2

This patch is using the patch from bug 557620 so I'm asking Olli to review it too.
Attachment #437880 - Flags: review?(Olli.Pettay)

Comment 18

7 years ago
(In reply to comment #5)
> CCing:
> - David for accessibility. This new input state may interest you.

What UI changes are going to be introduced? We should ensure new UI is accessible. 

Also it's worth to get Marco for testing when patch will be ready.
(Assignee)

Comment 19

7 years ago
The UI is probably going to look like the searchbox. The decision will probably be taken in bug 558594.
Comment on attachment 437878 [details] [diff] [review]
Tests v2

Would be great to add some tests for context menus etc. That could be done in a separate bug, so that you could test all the new input types there.
Attachment #437878 - Flags: review?(Olli.Pettay) → review+
Attachment #437880 - Flags: review?(Olli.Pettay) → review+
Comment on attachment 437880 [details] [diff] [review]
Patch v2


> PRBool
> nsGenericHTMLFormElement::IsTextControl(PRBool aExcludePassword) const
> {
>   PRInt32 type = GetType();
>   return type == NS_FORM_INPUT_TEXT ||
>+         type == NS_FORM_INPUT_SEARCH ||
>          type == NS_FORM_INPUT_TELEPHONE ||
>          (!aExcludePassword && type == NS_FORM_INPUT_PASSWORD) ||
>          type == NS_FORM_TEXTAREA;
> }
> 
> PRBool
> nsGenericHTMLFormElement::IsSingleLineTextControl(PRBool aExcludePassword) const
> {
>   PRInt32 type = GetType();
>   return type == NS_FORM_INPUT_TEXT ||
>+         type == NS_FORM_INPUT_SEARCH ||
>          type == NS_FORM_INPUT_TELEPHONE ||
>          (!aExcludePassword && type == NS_FORM_INPUT_PASSWORD);
> }

Perhaps you could add a simple static method which these methods would use.
That way you wouldn't need to add new form types to both these methods.
You could do that in the "tel" patch or here.
(Assignee)

Updated

7 years ago
Blocks: 559309
(Assignee)

Comment 22

7 years ago
(In reply to comment #20)
> (From update of attachment 437878 [details] [diff] [review])
> Would be great to add some tests for context menus etc. That could be done in a
> separate bug, so that you could test all the new input types there.

bug 558651 and bug 559309 should cover that.

(In reply to comment #21)
> (From update of attachment 437880 [details] [diff] [review])
> 
> > PRBool
> > nsGenericHTMLFormElement::IsTextControl(PRBool aExcludePassword) const
> > {
> >   PRInt32 type = GetType();
> >   return type == NS_FORM_INPUT_TEXT ||
> >+         type == NS_FORM_INPUT_SEARCH ||
> >          type == NS_FORM_INPUT_TELEPHONE ||
> >          (!aExcludePassword && type == NS_FORM_INPUT_PASSWORD) ||
> >          type == NS_FORM_TEXTAREA;
> > }
> > 
> > PRBool
> > nsGenericHTMLFormElement::IsSingleLineTextControl(PRBool aExcludePassword) const
> > {
> >   PRInt32 type = GetType();
> >   return type == NS_FORM_INPUT_TEXT ||
> >+         type == NS_FORM_INPUT_SEARCH ||
> >          type == NS_FORM_INPUT_TELEPHONE ||
> >          (!aExcludePassword && type == NS_FORM_INPUT_PASSWORD);
> > }
> 
> Perhaps you could add a simple static method which these methods would use.
> That way you wouldn't need to add new form types to both these methods.
> You could do that in the "tel" patch or here.

I did that in 'tel' patch.
(Assignee)

Comment 23

7 years ago
Created attachment 438984 [details] [diff] [review]
Patch v2.1

This is is updated to conform to 'tel' patch.
Jonas, I'm asking you to sr it because it should be quite trivial to do that after the sr of the 'tel' patch. Do not hesitate to cancel the sr or give it to someone else if you do not have enough time.
Attachment #437880 - Attachment is obsolete: true
Attachment #438984 - Flags: superreview?(jonas)
Attachment #438984 - Flags: superreview?(jonas) → superreview+
(Assignee)

Comment 24

7 years ago
Thank you for the reviews.
Now we need to wait for bug 551670 and bug 557620 to be landed before asking for a checkin.
I will probably work on the UI in a day or two.

Updated

7 years ago
Blocks: 559747
(Assignee)

Updated

7 years ago
Blocks: 345822
(Assignee)

Updated

7 years ago
Blocks: 562625
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
(Assignee)

Comment 25

7 years ago
r=smaug, sr=sicking
(Assignee)

Comment 26

7 years ago
Created attachment 444858 [details] [diff] [review]
Tests v2.1

r=smaug

Updated patch to apply correctly in the current tree.
Attachment #437878 - Attachment is obsolete: true
(Assignee)

Comment 27

7 years ago
Created attachment 444859 [details] [diff] [review]
Patch v2.2

r=smaug sr=sicking

Updated patch to apply correctly in the current tree.
Attachment #438984 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/f7a1cdc9d708
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
comment28 was the test, this is the patch:
http://hg.mozilla.org/mozilla-central/rev/ac8653880111
(Assignee)

Comment 30

7 years ago
Thank you Marco :)
Flags: in-testsuite+

Updated

7 years ago
Blocks: 566348

Comment 31

7 years ago
(In reply to comment #29)
> http://hg.mozilla.org/mozilla-central/rev/ac8653880111

With this patch you're no longer able to write in the Filter-section at about:config from SeaMonkey-Trunk.

Confirmed bei Hartmut Figge in the german nighty-builds-newsgroup by backing out the patch. MID: <4BF46D8F.6030903@hfigge.myfqdn.de>
(Assignee)

Comment 32

7 years ago
(In reply to comment #31)
> (In reply to comment #29)
> > http://hg.mozilla.org/mozilla-central/rev/ac8653880111
> 
> With this patch you're no longer able to write in the Filter-section at
> about:config from SeaMonkey-Trunk.
> 
> Confirmed bei Hartmut Figge in the german nighty-builds-newsgroup by backing
> out the patch. MID: <4BF46D8F.6030903@hfigge.myfqdn.de>

Could you open a follow-up and attach the changes made by Hartmut Figge ? A link to the thread would be fine too.

Comment 33

7 years ago
(In reply to comment #32)

> > Confirmed bei Hartmut Figge in the german nighty-builds-newsgroup by backing
> > out the patch. MID: <4BF46D8F.6030903@hfigge.myfqdn.de>
> 
> Could you open a follow-up and attach the changes made by Hartmut Figge ?

Hartmut only backed out Marco's patch from https://bugzilla.mozilla.org/show_bug.cgi?id=456229#c28 : http://hg.mozilla.org/mozilla-central/rev/ac8653880111 and compiled a new SeaMonkey with the former code.

> link to the thread would be fine too.

Message-ID <4BF44FD2.3020407@hfigge.myfqdn.de> or http://de.nntp2http.com/comm/software/mozilla/nightly-builds/2010/05/5fd901e2ec8183722b8a393d0ac604e5.html

Comment 34

7 years ago
(In reply to comment #33)

> https://bugzilla.mozilla.org/show_bug.cgi?id=456229#c28

Sorry, https://bugzilla.mozilla.org/show_bug.cgi?id=456229#c29 was right.

Comment 35

7 years ago
(In reply to comment #32)

> Could you open a follow-up and attach the changes made by Hartmut Figge ?

First there was the regression range provided by Ruediger. Jens then showed the checkins for this timeframe.

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2010-05-16+19%3A54%3A33&enddate=2010-05-17+23%3A12%3A28

http://hg.mozilla.org/mozilla-central/rev/ac8653880111 seemed to be worth a try, so i backed it out. The error was gone. An immediately following new build without outbacking showed the error again.
(Assignee)

Comment 36

7 years ago
(In reply to comment #35)
> (In reply to comment #32)
> 
> > Could you open a follow-up and attach the changes made by Hartmut Figge ?
> 
> First there was the regression range provided by Ruediger. Jens then showed the
> checkins for this timeframe.
> 
> http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2010-05-16+19%3A54%3A33&enddate=2010-05-17+23%3A12%3A28
> 
> http://hg.mozilla.org/mozilla-central/rev/ac8653880111 seemed to be worth a
> try, so i backed it out. The error was gone. An immediately following new build
> without outbacking showed the error again.

That does not really help. Is the about:config field a <input type='search'> in Seamonkey ? Or the error is maybe more vicious. It should be great to find what part of the part is making about:config failing so we can fix it.
Could you investigate that ?

Comment 37

7 years ago
Created attachment 446605 [details]
about:config-Filter with DOM-I

Comment 38

7 years ago
(In reply to comment #37)
> Created an attachment (id=446605) [details]
> about:config-Filter with DOM-I

To comment that screenshot:

I talked about the Filterfield in about:config. about:config himself works fine. You only can't type anything in the filter-field.
And yes, this field looks like a <input type='search'>
(In reply to comment #31)
> (In reply to comment #29)
> > http://hg.mozilla.org/mozilla-central/rev/ac8653880111
> 
> With this patch you're no longer able to write in the Filter-section at
> about:config from SeaMonkey-Trunk.

That regression should have been fixed through bug 566716 (cf. bug 566716 comment 3). Note that SM uses the same file for about:config as FF (config.xul) but SM's Type Ahead Find was interfering.
(Assignee)

Comment 40

7 years ago
(In reply to comment #39)
> (In reply to comment #31)
> > (In reply to comment #29)
> > > http://hg.mozilla.org/mozilla-central/rev/ac8653880111
> > 
> > With this patch you're no longer able to write in the Filter-section at
> > about:config from SeaMonkey-Trunk.
> 
> That regression should have been fixed through bug 566716 (cf. bug 566716
> comment 3). Note that SM uses the same file for about:config as FF (config.xul)
> but SM's Type Ahead Find was interfering.

Great ! Thank you for the information Jens :)

Comment 41

7 years ago
(In reply to comment #39)

> That regression should have been fixed through bug 566716

Indeed, it is fixed now as my fresh build shows. :)

Comment 42

7 years ago
(In reply to comment #41)

> Indeed, it is fixed now as my fresh build shows. :)

I can confirm that too. Thanks.
Depends on: 570230
Janet documented this:

https://developer.mozilla.org/en/HTML/Element/input
Keywords: dev-doc-needed → dev-doc-complete

Updated

7 years ago
Blocks: 581596

Updated

3 years ago
Blocks: 1055085
You need to log in before you can comment on or make changes to this bug.