Closed Bug 83827 Opened 24 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: