Closed Bug 83827 Opened 23 years ago Closed 22 years ago

When adding bookmark, "New Folder..." should select "New folder" text for user

Categories

(SeaMonkey :: Bookmarks & History, defect, P5)

Product:
SeaMonkey
Component:
Bookmarks & History
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Future

People

(Reporter: kbh7, Assigned: bugs)

References

Details

(Keywords: polish)

Attachments

(1 file, 1 obsolete file)

New patch to select text in single field dialogs
555 bytes, patch
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.2.19 i686; en-US; rv:0.9+) Gecko/20010601
BuildID:    2001060121

When adding a bookmark, the "New Folder..." dialog should immediately
select and give focus to the text entry box ("New Folder"), so the
user doesn't have to click there to type the folder name.

Reproducible: Always
Steps to Reproduce:
1. Right-click on a web page, and "Bookmark this Page".
2. Click on "New Folder..."
3. Start typing a folder name.

Actual Results:  Nothing happens -- the text entry box doesn't have focus.

Expected Results:  The text entry box should start out with focus, and the text
there ("New folder") should be selected, so users can just
start typing.
For me the textbox is focused but not selected, which is pretty bad if you ask
me since nobody will want to keep "new folder" as a name.
Ben, this is just a matter of changing .focus() to .select()
r=fabian on the patch haha ;-)
Status: UNCONFIRMED → NEW
Ever confirmed: true
seeing this as well linux cvs from today
shoot me, I suck (and it's only 1AM), the stuff I proposed has nothing to do
with this
this stuff uses the prompt code and it works on windows but not on linux.
cc'ing ccarlen. ccarlen is this a known issue?
On Mac as well as Windows, the edit field has the focus when the dialog comes
up. I also wish that, in addition to giving focus to the edit field, it would
select all the text in it. And, yes, this does use the prompt code. However, the
prompt code does nothing with setting the focus - it's at the mercy of
commonDialog.xul which it uses to do the dialog. 
my guess is that we need to change all the .focus() to .select() in
commonDialog.js function commonDialogOnLoad. But the mistery remains: why does
.focus() do its job on win and mac and not on linux currently. 
*** Bug 85252 has been marked as a duplicate of this bug. ***
OS: Linux → All
Status: NEW → ASSIGNED
Target Milestone: --- → Future
I managed to fix this, I'm going to try installing that patch maker soon.
Anyways, in commonDialog.js in function commonDialogOnLoad() go right down to 
the bottom of it where the .focus() is.  All I did was added a .select() before 
it.

Now, I don't know if this will mess up any other dialog boxes.  I'll do a little 
testing.
Okay, I checked through mozilla.  The only places that I found which could be 
using the common dialog are:

View->Text Zoom->Zoom Other
Preferences->Mail & News->Send Format->Other
And when setting the title of the www page in composer.

Zoom other starts off with 300% in the textbox selected all the time (With and 
without the .select()) which suggests that it doesn't use a common dialog even 
though it could.  And the other 2 don't have any text in them when they start 
off, so it's not selecting anything.  So, from what I can tell it doesn't mess 
up anything.
the dialogs like alert, prompt, confirm, etc, also use the common dialog.
Is the behavior still the same as I described earlier in this bug? i.e. it works
for Mac and Win but not for Linux?
The only code that I changed was the 1 textbox with 2 buttons case.  So the 
others should remain unaffected.  I only have windows boxes here so I don't know 
about linux.
Blocks: patchmaker
Paul Chen is now taking Bookmarks bugs. For your convenience, you can filter 
email notifications caused by this by searching for 'ilikegoats'.
Assignee: ben → pchen
Status: ASSIGNED → NEW
Mass move Ben's bugs dumped on me marked future with p5 to get off my untriaged
radar. You can filter out this email by looking for "ironstomachaussie"
Priority: -- → P5
Blocks: 49574
Added patch keyword. 
pchen, any chance for a r= ?
Keywords: patch, review
Comment on attachment 53793 [details] [diff] [review]
Selects the text in the commondialog.

We have to make sure this is the desired behavior for *all* the dialogs that
use commonDialog.js, and I suspect there are some of them (password dialogs,
prompts, ...). If that is is verified, r=fabian.
Attachment #53793 - Flags: review+
The patch only affects common dialogs which use one input box.  I did not change 
the case for username/password dialog boxes.

There are currently only a few places where the one textbox common dialog is 
used (See comment #9) and in those cases the change doesn't affect them because 
there is no text to select.

In the case of Bug 49574 selecting all of the text is a wanted result.
I want to see a list of dialogs that use this for verification before I sr=. LXR
is your friend.
If I could figure out how to use lxr, I would give you a better list.  I don't
know how to search for lines containing "prompt-service" followed closely with a
line which contains ".prompt(", so the best I could do was this:

http://lxr.mozilla.org/mozilla/search?string=%5C.prompt%28

I don't know why this doesn't pickup chatzilla, but out of that search, these
are the files which, I believe, the patch affects.

http://lxr.mozilla.org/mozilla/source/editor/ui/composer/content/ComposerCommands.js#479
http://lxr.mozilla.org/mozilla/source/mailnews/compose/prefs/resources/content/pref-formatting.js#76
http://lxr.mozilla.org/mozilla/source/mailnews/compose/resources/content/MsgComposeCommands.js#1527
http://lxr.mozilla.org/mozilla/source/mailnews/compose/resources/content/MsgComposeCommands.js#2069
http://lxr.mozilla.org/mozilla/source/profile/resources/content/profileManager.js#91
http://lxr.mozilla.org/mozilla/source/xpfe/browser/resources/content/personalToolbar.js#152
http://lxr.mozilla.org/mozilla/source/xpfe/components/search/resources/search-editor.js#513
http://lxr.mozilla.org/mozilla/source/xpfe/components/search/resources/search-editor.js#461
http://lxr.mozilla.org/mozilla/source/xpfe/components/bookmarks/resources/bookmarksTree.js#270

The only place I can see it conflicting is with the Attach Web Page in the
message compose dialog.  There it has http:// already written in the prompt
dialog box and this code would select the http:// text.  Of course in the same
window, if I try sending it without a subject, it comes up with a prompt saying
(No subject) which I would have to go back and delete before being able to type.

mass reassign of pchen bookmark bugs to ben
Assignee: pchen → ben
I hope somebody picks this up and does something about it... it's a minor issue,
but still an annoyance.  I never want to call my new bookmark folders "New
Folder", but am forced to highlight that text first before I can type something
else.  If it's not feasible for some reason to make that text highlighted when
it comes up, it would be better to start new folders with blank text instead of
"New Folder".
*** Bug 121418 has been marked as a duplicate of this bug. ***
Attachment #53793 - Attachment is obsolete: true
Comment on attachment 53793 [details] [diff] [review]
Selects the text in the commondialog.

This patch no longer applies (the file does
not have a section where it focuses the field)...

Should thos not now be handled by dialog.xml
anyway?  If so, that file will need to be patched.

If the patch is updated, please mark this as blocking
bug 123569
It appears that the changes in bug 114580 impacted this patch.
I still don't see it working.  (I'm not about to make another patch either. 
I've been turned off of developing patches for mozilla - they never get
accepted.  Someone else can waste their time.)
This is the first patch I've ever written for Mozilla, so cut me some slack if
this isn't the best place to put this code. It fixes the lack of text selection
for this specific bug as well as the JavaScript prompt selection problem.
This patch also has the problem with the "Attach Web Page"-dialog in message
compose that Neil mentioned in comment #19.


I don't see it as a "problem".  I just said that that is what happened.  I find
nothing wrong with selecting the text.
Marking as blocker of bug 123569 as requested by Boris Zbarsky. Can we get this 
reviewed, please? This is a minor change, but impacts a lot of dialogs, 
including any use of JavaScript prompt() with default text (see bug 62880 which 
was marked as a duplicate of bug 28583.)
Blocks: 123569
Keywords: mozilla1.0
*** Bug 143428 has been marked as a duplicate of this bug. ***
*** Bug 143374 has been marked as a duplicate of this bug. ***
Note, the Dom Inspector change value prompt should also have the value selected 
when the dialog is spawned.
*** Bug 152232 has been marked as a duplicate of this bug. ***
*** Bug 152835 has been marked as a duplicate of this bug. ***
URL: any
Keywords: polish
Hardware: PC → All
*** Bug 162920 has been marked as a duplicate of this bug. ***
This seems to be fixed in trunk build 2002101408 on Win2k.
Not fixed yet in 2002090206 on Win2k; some comments suggest that it has been
working on Win, but it has *not* - at least for more than some months.
Bug 49574, which is blocked by this bug, seems also fixed.
Can anyone check this?
Fixed in 1.2b
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
No longer blocks: patchmaker
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: