Closed
Bug 245224
Opened 21 years ago
Closed 18 years ago
Selected attribute of <tab> doesn't work
Categories
(Core :: XUL, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: surkov, Assigned: jag+mozilla)
References
Details
Attachments
(3 files, 4 obsolete files)
(deleted),
application/vnd.mozilla.xul+xml
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/plain
|
Details |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040316
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040316
If I set selected="true" attribute on the tab element then this tab isnt' selected
Reproducible: Always
Steps to Reproduce:
Actual Results:
Tab with selected="true" attribute isn't selected
Expected Results:
Tab with selected="true" attribute should be selected
Reporter | ||
Comment 1•21 years ago
|
||
Comment 2•21 years ago
|
||
This doesn't work in builds going back to 1.6 or so, at least... According to
xulplanet it should work. Neil, any idea whether this should be working?
Comment 4•21 years ago
|
||
Confirming with a Linux build from yesterday. I couldn't find a dupe.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
Summary: selected attribute of tab doesn't work → Selected attribute of <tab> doesn't work
Whiteboard: DUPEME
Reporter | ||
Comment 5•20 years ago
|
||
I offer to extend interface of tabbox to improve possibilities of tabbox. Also
this improvments help to fix this bug easy.
1) I suggest to fire select event for the tab element (not tabs as before)
1) I suggest to fire unselect event for the tab element
2) I suggest to do in order to selected property of tab selects tab.
Reporter | ||
Comment 6•20 years ago
|
||
I offer to extend interface of tabbox to improve possibilities of tabbox. Also
this improvments help to fix this bug easy.
1) I suggest to fire select event for the tab element (not tabs as before)
1) I suggest to fire unselect event for the tab element
2) I suggest to do in order to selected property of tab selects tab.
Reporter | ||
Comment 7•20 years ago
|
||
to comment#6: in attachment need to replace name of new method with _selectByIndex
Reporter | ||
Updated•20 years ago
|
Attachment #168001 -
Attachment is obsolete: true
Reporter | ||
Comment 8•20 years ago
|
||
selected property of tab should have only getter (third suggestion of comment #6
is wrong):
<property name="selected" readonly="true"
onget="return this.getAttribute('selected') == 'true' ? true : false;"/>
Comment 9•20 years ago
|
||
I disagree with all three suggestions of comment 6. Select events always fire on
the container element as they do with html <select> elements. As for comment 8,
the setter is a convenience for the <tabs> element, but there's no direct way to
enforce privacy except by writing tabs to use setAttribute everywhere.
Reporter | ||
Comment 10•20 years ago
|
||
If select event will be dispatched to tab element then it will bubbled to tabs
element. If target of select event must be container then select and unselect
events may fire for container. In this case we can place handlers of select and
unselect elements on container and we can put "afterselected" and
"beforeselected" attributes in these handlers.
It's needed to add method (_selectByIndex() in attachment) like setter of
selectedIndex (without check of selected item) to fix this bug.
Comment 11•20 years ago
|
||
Now I remember why we permit setting .selected on a tab - tabbrowser needs to do
that on a tab just before it removes it.
To fix this bug it might be simple enough to do this:
try {
this.selectedTab.selected = true;
} catch (e) {
this.selectedIndex = 0;
}
Comment 12•20 years ago
|
||
Sorry, I mean this.selectedItem of course.
Reporter | ||
Comment 13•20 years ago
|
||
Ok. This fix is nicer.
About select events. <html:select> and <xul:menulist> elements doesn't fire
select event. Why should select event be fired for the container?
What do you think about unselect event? I seem it is needed for tab
unitializing. Is there a way to know previous selected tab?
Comment 14•20 years ago
|
||
Sorry, maybe I was thinking about the change event.
I can't remember why we fire select events, I'm guessing accessibility, in which
case aaron will know why we don't fire them on menulists.
Comment 15•20 years ago
|
||
(In reply to comment #14)
> Sorry, maybe I was thinking about the change event.
> I can't remember why we fire select events, I'm guessing accessibility, in which
> case aaron will know why we don't fire them on menulists.
For menulists, accessibility uses an event called "DOMMenuItemActive". I can't
remember why we don't just use "select" within a menu. It was a few years ago
when David Hyatt asked me to call it DOMMenuItemActive. I can't think of any
reason to call it that instead of "select", but we'd need to change the
accessibility mapping code.
Reporter | ||
Comment 16•20 years ago
|
||
To comment #11. I think It's wrong way to fix this bug because tab will be
selected but select event will not be fired.
Comment 17•20 years ago
|
||
Why do you think setting the selected tab wouldn't fire the select event?
Reporter | ||
Comment 18•20 years ago
|
||
Because this.selectedItem.selected=true don't fire select event. I mean tab
selection on initialization when selected attribute is setted on the tab.
Comment 19•20 years ago
|
||
Sorry, I'd already forgotton what I'd written...
How about this.selectedItem = this.selectedItem; ?
Reporter | ||
Comment 20•20 years ago
|
||
Yes. It will fix the bug.
Comment 21•20 years ago
|
||
Updated•20 years ago
|
Attachment #168134 -
Flags: superreview?(jag)
Attachment #168134 -
Flags: review?(mconnor)
Reporter | ||
Comment 22•20 years ago
|
||
Sorry. I was wrong. This patch doesnt' fix the problem because selectedItem
doesn't select selected element.
Comment 23•20 years ago
|
||
Comment on attachment 168134 [details] [diff] [review]
Patch to respect initially selected tab
You're right, I'm not thinking straight. In fact, if the first tab has
selected="true" even the old code doesn't work.
Attachment #168134 -
Flags: superreview?(jag)
Attachment #168134 -
Flags: review?(mconnor)
Reporter | ||
Comment 24•20 years ago
|
||
I can see only one way: add some method witch will select element in any way as
I propose before.
Comment 25•20 years ago
|
||
Can't we either
1) move the part of tabs.selectedIndex setter after the !tabs[val].selected to a
separate method and call that from tabs constructor (comment 10)
-or-
2) do something like either a removeAttribute("selected") or selected=false on
the tab we need to select before calling:
var selIdx;
try {
selIdx = this.selectedIndex;
this.tabs[selIdx].removeAttribute("selected"); /*or .selected=false but both
are ugly*/
} catch(e) {
selIdx = 0; //[*]
}
this.selectedIndex = selIdx;
Also keep in mind that there's also bug 256024, and to fix it we also need to
replace [*] with code that checks tabpanels' selectedIndex attribute.
Reporter | ||
Comment 26•20 years ago
|
||
I think the second way isn't nice unlike the first way. I dont consider
unselecting and subsequent selecting of tab as the right way. It's better to
modify tabs.selectedIndex setter.
Comment 27•20 years ago
|
||
How about this version? Note that it never fires an initial select event.
Attachment #168134 -
Attachment is obsolete: true
Comment 28•20 years ago
|
||
I don't think it will select correct panel on startup.
And I'm not sure if not firing select event is cool.
Comment 29•20 years ago
|
||
Ah yes, this method doesn't select the correct tabpanel.
Comment 30•20 years ago
|
||
I'm still not convinced we need a select event.
Updated•20 years ago
|
Attachment #169710 -
Attachment is obsolete: true
Comment 31•20 years ago
|
||
>I'm still not convinced we need a select event.
I still think it would be more sane to fire initial select event (If nothing
else, there was a reson for it to be fired before, I suppose). But it doesn't
really matter much to me.
The fact it sets the selected tab in tabs constructor and the selected panel in
tabbox constructor is quite confusing and at least deserving a comment, imo.
(btw pardon my ignorance, are you sure constructors are always called in correct
order?)
Reporter | ||
Comment 32•20 years ago
|
||
Why don't you want to do approximately the same like in attachment? It will
select intial tab and send message to initial tab.
Or do like in variant 2 of comment#25.
Attachment #168002 -
Attachment is obsolete: true
Reporter | ||
Comment 33•20 years ago
|
||
I am confused such simple bug is cause serious discussion. Need to select
initial tab and send message to it. Nothing more. It is no important by the
highest standarts what way will be choosed.
Comment 34•20 years ago
|
||
OK, I can see some benefits of your approach, but I'm still not convinced about
the initial event. For instance, an HTML <select size="1"> element doesn't send
an event when it auto-selects the first option.
Reporter | ||
Comment 35•20 years ago
|
||
Reasons why initial select event should be fired:
1. There is initial select event in the current tab realization (for element
with selectedIndex=0).
2.
2.1 I can't understand for what we need to segregate initial select from another
select events.
2.2 When I add handler on select event then I don't see a big difference how
event was rised (initial, by user or programatically). I want to do some actions
when tab is selected.
2.3 I can't find any example in witch I would be forced to use special
processing when I handle initial select event. More I guess it's not right to
use something special to process initial selec event. (You're right it's not
objective reason).
3. I think it is not exactly right to compare html:select and tabs elements.
Thease elements rise different events. Although onchange event (html:select
element) behaves like onselect event (tabs element) when selected item is
changed by user, but onchange event is not fired when selected item is changed
programatically unlike onselect event.
Comment 36•20 years ago
|
||
Ben Goodger recently checked in some changes to toolkit's tabbox which would
seem to allow the preselecting of the selected index in certain circumstances.
Comment 37•19 years ago
|
||
Setting the selected attribute on a <tab> shouldn't do anything anyway, so this is invalid. I'm not following the select event discussion though, is that a different issue?
Comment 38•19 years ago
|
||
This is not about setting the attribute from a script, but about a document with <tab selected="true"/>. XUL Planet's description implies this should work ("This attribute is set to true if the tab is selected by default.")
The select event discussion is about whether a select event should be fired on load when such tab is selected. Now that I think of it, seems to me neil was right when he suggested to _not_ fire the initial event.
Reporter | ||
Comment 39•19 years ago
|
||
(In reply to comment #38)
>
> The select event discussion is about whether a select event should be fired on
> load when such tab is selected. Now that I think of it, seems to me neil was
> right when he suggested to _not_ fire the initial event.
>
If you insist on that initial select shouldn't be fired then it's ok for me. Though it's vexed question for me as before.
Comment 40•19 years ago
|
||
(In reply to comment #38)
> This is not about setting the attribute from a script, but about a document
> with <tab selected="true"/>.
Yes, that should work. Setting it any other time should not change the selection.
Widgets should not fire the select event on initial selection.
Comment 41•18 years ago
|
||
Fix by bug 370742
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•