Last Comment Bug 452415 - Switch from <textbox type="timed"> to type="search" in SeaMonkey
: Switch from <textbox type="timed"> to type="search" in SeaMonkey
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: All All
-- normal (vote)
: ---
Assigned To: neil@parkwaycc.co.uk
:
:
Mentors:
: 125185 449690 (view as bug list)
Depends on: 388811 451722
Blocks: 449045 533956
  Show dependency treegraph
 
Reported: 2008-08-27 07:12 PDT by neil@parkwaycc.co.uk
Modified: 2009-12-10 06:59 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch for suite/ (3.36 KB, patch)
2008-08-27 07:21 PDT, neil@parkwaycc.co.uk
kairo: review+
Details | Diff | Splinter Review
Patch for mailnews/addrbook/ (3.36 KB, patch)
2008-08-27 07:38 PDT, neil@parkwaycc.co.uk
no flags Details | Diff | Splinter Review
Patch for mailnews/base/ (2.80 KB, patch)
2008-08-27 08:39 PDT, neil@parkwaycc.co.uk
mnyromyr: review+
Details | Diff | Splinter Review
Patch for mailnews/addrbook/ (3.98 KB, patch)
2008-08-27 08:40 PDT, neil@parkwaycc.co.uk
standard8: review-
Details | Diff | Splinter Review
Revised patch for mailnews/addrbook/ (4.19 KB, patch)
2008-08-30 06:46 PDT, neil@parkwaycc.co.uk
standard8: review+
Details | Diff | Splinter Review

Description User image neil@parkwaycc.co.uk 2008-08-27 07:12:45 PDT
I understand that toolkit wants to get rid of type="timed" anyway.
Comment 1 User image neil@parkwaycc.co.uk 2008-08-27 07:21:26 PDT
Created attachment 335705 [details] [diff] [review]
Patch for suite/

Trying KaiRo for review because he wrote the applications pane.
Comment 2 User image neil@parkwaycc.co.uk 2008-08-27 07:38:16 PDT
Created attachment 335709 [details] [diff] [review]
Patch for mailnews/addrbook/

Interestingly the main clear button entities are reused from messenger.dtd and Thunderbird has a different sidebar and no addressing dialog.
Comment 3 User image neil@parkwaycc.co.uk 2008-08-27 08:39:01 PDT
Created attachment 335715 [details] [diff] [review]
Patch for mailnews/base/

...since subscribe.xul is still shared, right?
Comment 4 User image neil@parkwaycc.co.uk 2008-08-27 08:40:34 PDT
Created attachment 335716 [details] [diff] [review]
Patch for mailnews/addrbook/

The real patch this time...
Comment 5 User image David :Bienvenu 2008-08-27 08:54:41 PDT
Comment on attachment 335715 [details] [diff] [review]
Patch for mailnews/base/

yes, afaik it's shared. TB doesn't have its own.
Will the timeout for quick search change now?
Comment 6 User image Phil Ringnalda (:philor) 2008-08-27 08:56:29 PDT
Comment on attachment 335715 [details] [diff] [review]
Patch for mailnews/base/

Nope, not shared - I already changed our fork, though I didn't have quite enough nerve to drop the 300ms timeout even though I don't have any idea how it was chosen.
Comment 7 User image Phil Ringnalda (:philor) 2008-08-27 09:14:03 PDT
Comment on attachment 335715 [details] [diff] [review]
Patch for mailnews/base/

Which, fortunately, means you don't need the sr that you had before I midaired it.
Comment 8 User image Mark Banner (:standard8) 2008-08-29 03:05:59 PDT
Comment on attachment 335716 [details] [diff] [review]
Patch for mailnews/addrbook/

>diff -r 4b62be6f26a1 mailnews/addrbook/resources/content/abSelectAddressesDialog.xul
>--- a/mailnews/addrbook/resources/content/abSelectAddressesDialog.xul	Tue Aug 26 22:40:20 2008 +0100
>+++ b/mailnews/addrbook/resources/content/abSelectAddressesDialog.xul	Wed Aug 27 16:39:30 2008 +0100
>@@ -81,11 +81,9 @@
>         <menupopup id="addressbookList-menupopup" class="addrbooksPopup"/>
>       </menulist>  
>       <label value="&for.label;" accesskey="&for.accesskey;" control="searchInput"/>
>-      <textbox id="searchInput" flex="1"
>-               type="timed" timeout="800"
>+      <textbox id="searchInput" flex="1" type="search"
>                oninput="SearchInputChanged();"
>                oncommand="onEnterInSearchBar();"/>  
>-      <button id="clear" label="&clearButton.label;" disabled="true" accesskey="&clearButton.accesskey;" oncommand="onAbClearSearch();"/>
>     </hbox>

If I understand this right, I believe we don't need onAbClearSearch except for addressbook-panel (which requires it for compose-window-close notifications), so I think we should be moving that function from abCommon.js to addressbook-panel.js.

>diff -r 4b62be6f26a1 mailnews/addrbook/resources/content/addressbook.xul
>--- a/mailnews/addrbook/resources/content/addressbook.xul	Tue Aug 26 22:40:20 2008 +0100
>+++ b/mailnews/addrbook/resources/content/addressbook.xul	Wed Aug 27 16:39:30 2008 +0100
>@@ -246,7 +246,8 @@
>                       accesskey="&propertiesCmd.accesskey;"
>                       key="key_properties"
>                       command="cmd_properties"/>
>-            <menuitem id="menu_preferences" oncommand="goPreferences('mailnews', 'chrome://messenger/content/addressbook/pref-addressing.xul', 'mailaddressbookpref')"/>
>+            <menuitem id="menu_preferences"
>+                      oncommand="goPreferences('addressing_pane')"/>
>           </menupopup>
>         </menu>
>         <menu id="menu_View">

This is a different bug...

r=me with those fixed.

Also, all the bits here are TB-specific, so you shouldn't need Dan as an sr.
Comment 9 User image Mark Banner (:standard8) 2008-08-29 03:18:35 PDT
Comment on attachment 335716 [details] [diff] [review]
Patch for mailnews/addrbook/

I forgot, I'm getting the following error on searching in the select addresses dialog:

JavaScript error: chrome://messenger/content/addressbook/abCommon.js, line 636: gSearchInput.nextSibling is null
Comment 10 User image Karsten Düsterloh 2008-08-29 16:21:21 PDT
Comment on attachment 335715 [details] [diff] [review]
Patch for mailnews/base/

Now we just need TB's search term context menu, too, for extra coolness. ;-)
Comment 11 User image neil@parkwaycc.co.uk 2008-08-30 06:46:40 PDT
Created attachment 336174 [details] [diff] [review]
Revised patch for mailnews/addrbook/
Comment 12 User image Robert Kaiser 2008-08-31 06:17:03 PDT
Comment on attachment 335705 [details] [diff] [review]
Patch for suite/

From what I see, it seems to work alright. I'm too little expert on the code, but the patch looks alright to me.
Comment 13 User image Mark Banner (:standard8) 2008-09-01 04:21:50 PDT
Comment on attachment 336174 [details] [diff] [review]
Revised patch for mailnews/addrbook/

r=me if you put the abSelectAddressesDialog.dtd changes back in.
Comment 14 User image neil@parkwaycc.co.uk 2008-09-01 05:33:57 PDT
Pushed changeset ebd2167a2511 to comm-central.
Comment 15 User image Magnus Melin 2008-10-17 12:51:17 PDT
*** Bug 125185 has been marked as a duplicate of this bug. ***
Comment 16 User image Dão Gottwald [:dao] 2009-01-01 04:26:40 PST
*** Bug 449690 has been marked as a duplicate of this bug. ***

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