Closed
Bug 62690
Opened 24 years ago
Closed 24 years ago
Language picker in preferences dialog ignores double clicks
Categories
(SeaMonkey :: Preferences, defect, P3)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: cesarb, Assigned: shanjian)
Details
(Keywords: regression)
Attachments
(5 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux 2.4.0-test10 i686; en-US; m18) Gecko/20001211
BuildID: 2000121112
It used to be (2000120808 and before) that double-clicking on a language in the
list of languages to add would add that language. Now, double clicks there seems
to be treated as single clicks; to add a language, I have to click and then
press "OK".
Reproducible: Didn't try
Steps to Reproduce:
1. Do a fresh install of mozilla
2. Open Preferences, Languages, and click Add
3. Try to double-click in pt-BR
Actual Results: Selected pt-BR
Expected Results: Added pt-BR to the languages list
Using modern skin.
Comment 1•24 years ago
|
||
confirming... not sure if this is a toolkit or i18n issue. jrgm, d'you know if
this is a known issue, trying to select from list in a dialog by double-clicking
[thus dismissing the dialog and adding the selection to the parent
dialog/window]?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 2•24 years ago
|
||
Um, I don't see double-clicks doing anything in a build before 1208. Looking
(briefly) at the code in ./pref-languages[-add].(js|xul), there is no handler
defined or registered and I don't see that this code has changed in a while.
Since double-click works in other trees, this just looks like a bit of UI
with no implementation (perhaps none was intended to be added, but adding the
double click would be a (small) gain in usability). Who owns this dialog?
Naoki:
This is the same type Accept-Language UI bug we discussed earlier. Whoever owns
Jurai's bugs now will own this one, too. Back to you :-)
Assignee: tao → nhotta
Comment 6•24 years ago
|
||
Linux only? I don't see this problem with Windows RTM.
Status: NEW → ASSIGNED
Comment 7•24 years ago
|
||
I mean on windows double-click does not add the language to the list.
Comment 8•24 years ago
|
||
The assertion in the initial bug report was that double-click _used_ to add
the treeitem to the active list. [I don't believe that's true (see above),
but we haven't heard back from cesarb].
Nevertheless, it's still a reasonable bit of UI -- that double-click should
add the selected item to the active list, instead of doing nothing. So, it's
an RFE.
Reporter | ||
Comment 9•24 years ago
|
||
Yes, it used to work. That's how I found the problem - I do a clean install of
mozilla every time I install a new nightly, and follow the same sequence of
steps every time to configure it to the way I like. This includes adding pt-br
and pt to the list of languages, and I got used to do that by double-clicking.
Comment 10•24 years ago
|
||
Shanjian, could you take a look at this.
I think you are making changes to this currently, please try if we can enable
double-click to add a language.
Assignee: nhotta → shanjian
Status: ASSIGNED → NEW
Updated•24 years ago
|
Keywords: regression
Assignee | ||
Comment 11•24 years ago
|
||
Assignee | ||
Comment 12•24 years ago
|
||
Comment 14•24 years ago
|
||
+ var Languagename = new String();
+ var Languageid = new String();
Are these necessary? Can you just assign from getAttribute?
+
+ Languagename = node.getAttribute('value');
+ Languageid = node.getAttribute('id');
Indent is not right.
Comment 15•24 years ago
|
||
Do you really want to do |window.close| after adding the item on the
double-click? More commonly, this form of UI allows a user to add multiple
items, and then call close manually. (Cheap opinions from the peanut gallery).
Assignee | ||
Comment 16•24 years ago
|
||
Naoki,
The 2 assignment might not be necessary, but they will not do any harm either.
John,
Yes, window.close is necessary here. In most cases, use just want to add one languages, and double
click is a shortcut only for this purpose. Otherwise they would click add after selecting multiple
items.
Comment 17•24 years ago
|
||
r=nhotta
>The 2 assignment might not be necessary
If not necessary then please simplify.
Comment 18•24 years ago
|
||
Matthew, what do you think of that double-click discussion?
+ function HandleDoubleClick(event, node)
The event is unused, please don't pass it in.
+ var Languagename = new String();
+ var Languageid = new String();
As Naoki mentioned, these are unnecessary. Please just do it directly. Also,
please follow our typical variable naming convention if you don't mind
(e.g. languageName instead of Languagename).
+ return true;
This is unnecessary.
Also, it looks like we don't need to retrieve the languageName unless
!LangAlreadyActive(Languageid), so please move that into the |if|
Thanks.
Comment 19•24 years ago
|
||
I can't see the relevant dialog. Double clicking should only close a dialog if
the widget you are manipulating is very likely to be the only control you need to
change before the dialog is closed. (An example of this is the Shut Down dialog
in Windows 9x, where there's nothing else to change besides the radio buttons.)
If this is a standalone dialog with just one tree widget (and `Add' and `Cancel'
buttons), then closing on double click is fine.
Assignee | ||
Comment 20•24 years ago
|
||
Assignee | ||
Comment 21•24 years ago
|
||
Assignee | ||
Comment 22•24 years ago
|
||
who can give me sr=?
Comment 23•24 years ago
|
||
r=nhotta for new patch, you can e-mail erik for a super review and cc to
reviewers@mozilla.org.
Comment 24•24 years ago
|
||
Thanks for making those changes. One more minor point that isn't worth
attaching a new patch...
! datasources="rdf:null" multiple="true" ondblclick="return
HandleDoubleClick(event.target)">
Generally, the only reason you'd want to return the value in an event handler
is to possibly return false and prevent the default action from happening.
Since that isn't the case here, this can really just be
ondblclick="HandleDoubleClick(event.target);" (with semicolon).
Thanks!
Comment 25•24 years ago
|
||
you don't need to do a getAttribute of 'id', just node.id works fine.
Also, it may be worth determining whether or not the event.target is the correct
type of node before going off and doing something with that data. It's possible
to double click on other parts of the tree (empty space, headers) and have
getAttribute return an empty string and have the code continue executing with
that data (which is probably undesirable).
Assignee | ||
Comment 26•24 years ago
|
||
Assignee | ||
Comment 27•24 years ago
|
||
Ben pointed out a good point in his comment. If I double click scroll bar area,
the function will put a undesired item to accepted languages list. Since all of
our language items has value associated with them, I am using value to verify
node. Not sure if there is any better way, but my current code sounds good
enough.
Comment 28•24 years ago
|
||
seems reasonable, a=ben
Assignee | ||
Comment 29•24 years ago
|
||
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Comment 30•23 years ago
|
||
vrfy fixed on the 3 main platforms using comm branch bits, 2001.06.07.0x.
Status: RESOLVED → VERIFIED
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•