Closed
Bug 603141
Opened 14 years ago
Closed 14 years ago
Dropdown is not rendered
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla2.0b8
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta8+ |
People
(Reporter: pfinette, Assigned: mounir)
References
()
Details
(Keywords: dev-doc-complete, testcase)
Attachments
(2 files, 1 obsolete file)
3.63 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
3.28 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101009 Firefox/4.0b8pre Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101009 Firefox/4.0b8pre On this particular page the drop down for the shoe size is not rendered at all. Works in other browsers. Reproducible: Always Steps to Reproduce: 1. Go to URL 2. Dropdown should be next to image (where green arrow is) 3. No dropdown Actual Results: Nothing - there is no dropdown Expected Results: Render the dropdown
Comment 1•14 years ago
|
||
The dropdown at the site is plain HTML generated by MSDropDown, a jQuery plugin. (the site uses: MSDropDown 2.1, jQuery v1.4.2) An example dropdown looks something like this in Fx 3.6.x: <div style="width: 253px;" id="itemColorDDL_msdd" class="dd"> <div id="itemColorDDL_title" class="ddTitle"> <span id="itemColorDDL_arrow" class="arrow"></span> <span class="textTitle" id="itemColorDDL_titletext"> <img src="iconColorPicker.gif" align="left"> Choose Color</span> </div> <div style="width: 251px;" id="itemColorDDL_child" class="ddChild"> <a href="javascript:void(0);"> <img src="iconColorPicker.gif" align="left"> Choose Color</a> <a href="javascript:void(0);"> <img src="2009_Icon_Tarmac_Ventilated_Boots_Black.jpg"> Black</a> <a href="javascript:void(0);"> <img src="2009_Icon_Tarmac_Ventilated_Boots_White.jpg"> White</a> </div></div> In a trunk build the first child DIV "itemColorDDL_title" is missing. MSDropDown uses the following expression to decide if it should be generated or not (generates it when false): a(this).attr("size") > 0 || a(this).attr("multiple") == true In jQuery, a(this).attr("size") boils down to DOMElement["size"] on a real <select> (hidden with CSS). In Fx3.6.x we get zero if the "size" attribute isn't specified on the <select>, on trunk we get 1. Here's a testcase: data:text/html,<select></select><script>alert(document.getElementsByTagName('select')[0]["size"])</script> So, it appears MSDropDown/jQuery depends on a bug that we fixed.
Comment 2•14 years ago
|
||
This bug should probably go to Evangelism? The fix for the site is simple: specify size="1" on the <select>s. MSDropDown is from the following site? http://www.marghoobsuleman.com/jquery-image-dropdown
Comment 3•14 years ago
|
||
The sample at the MSDropDown site doesn't work in Fx4: http://www.marghoobsuleman.com/mywork/jcomponents/image-dropdown/samples/index.html and it uses the latest version - 2.3
Comment 4•14 years ago
|
||
(I left a note in the contact form at the MSDropDown site)
Assignee | ||
Updated•14 years ago
|
Component: General → DOM: Core & HTML
Product: Firefox → Core
QA Contact: general → general
Updated•14 years ago
|
Assignee: nobody → english-us
Blocks: 551846
Component: DOM: Core & HTML → English US
Product: Core → Tech Evangelism
QA Contact: general → english-us
Assignee | ||
Comment 5•14 years ago
|
||
So, the script should change the current condition from: a(this).attr("size") > 0 || a(this).attr("multiple") == true to: a(this).attr("size") > 1 || a(this).attr("multiple") == true Right?
![]() |
||
Comment 6•14 years ago
|
||
Do other browsers return 1 for size in this situation? If so, why does it work there? Sniffing on the part of this site?
Assignee | ||
Comment 7•14 years ago
|
||
(In reply to comment #6) > Do other browsers return 1 for size in this situation? If so, why does it work > there? Sniffing on the part of this site? They return 0 but as far as I understand the specs, it should be 1.
Assignee | ||
Comment 8•14 years ago
|
||
(In reply to comment #7) > (In reply to comment #6) > > Do other browsers return 1 for size in this situation? If so, why does it work > > there? Sniffing on the part of this site? > > They return 0 but as far as I understand the specs, it should be 1. Tested with Opera 10.7, Chrome 7 and Safari 5 (not tested with IE).
![]() |
||
Comment 9•14 years ago
|
||
IE8 returns 0 for the testcase in comment 1 at the end. If all browsers consistently return 0 and returning 1 leads to site breakage, we should just change the spec to match reality instead of making gratuitous changes, no? And go back to our old behavior?
Assignee: english-us → nobody
Component: English US → DOM: Core & HTML
Product: Tech Evangelism → Core
QA Contact: english-us → general
![]() |
||
Comment 10•14 years ago
|
||
Either way, deciding that needs to happen ASAP; ideally by b8. Mounir, do you want to raise this issue on the spec, or should I?
blocking2.0: --- → beta8+
Assignee | ||
Comment 11•14 years ago
|
||
I'm wondering if this change is breaking so many things. <select></select> and <select size='1'></select> should have the exact same behavior (and rendering). A clever check for "there is only one option shown and selectable" for Firefox 3.6, Opera, Chrome, Safari, IE should be: select.size <= 1 && !select.multiple With the new specifications (Firefox 4), it should be: select.size == 1 && !select.multiple So, there is no reason to check against 0 thus the retro-compatibility should be respected. Should we revert this change because a library was badly checking something and is now broken? This is a real question, I've no experience in retro-compatibility issues.
Assignee | ||
Comment 12•14 years ago
|
||
FWIW, this change is in the tree since 5 months and it's the first reported issue.
![]() |
||
Comment 13•14 years ago
|
||
> Should we revert this change because a library was badly checking something and
> is now broken?
It really depends on the context. Should we revert a change that made us different from every other browser _and_ broke at least one site? Yes, imo, unless there's an overwhelming case for making the change.
Comment 14•14 years ago
|
||
(In reply to comment #4) > (I left a note in the contact form at the MSDropDown site) Thank you Mats and all of you, I've joined this forum. I'll fix this. Give me some time. I am busy these days. Sorry for the inconvenience. Regards, MS
Comment 15•14 years ago
|
||
See spec bug: http://www.w3.org/Bugs/Public/show_bug.cgi?id=11162
Updated•14 years ago
|
Assignee | ||
Comment 16•14 years ago
|
||
I think we should move back to default to 0 and we will see what will happen with the specs.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #488901 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 17•14 years ago
|
||
Better with a refreshed patch...
Attachment #488901 -
Attachment is obsolete: true
Attachment #488902 -
Flags: review?(bzbarsky)
Attachment #488901 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•14 years ago
|
Version: unspecified → Trunk
![]() |
||
Comment 18•14 years ago
|
||
Do we want to stop returning 4 in the multiple case too? Why, if so?
Assignee | ||
Comment 19•14 years ago
|
||
(In reply to comment #18) > Do we want to stop returning 4 in the multiple case too? Why, if so? I think it would be confusing to default to 0 if multiple isn't set and 4 otherwise. Like Aryeh said in w3c bug (comment 15), I think we should default to 0 or 1/4 but certainly not 0/4.
![]() |
||
Comment 20•14 years ago
|
||
Comment on attachment 488902 [details] [diff] [review] Part 1 - Make select.size returns 0 by default Ok. Always zero was our old behavior, right? r=me
Attachment #488902 -
Flags: review?(bzbarsky) → review+
Comment 21•14 years ago
|
||
Fwiw, that's also what Chrome, Opera and IE8 does.
Assignee | ||
Comment 22•14 years ago
|
||
(In reply to comment #20) > Comment on attachment 488902 [details] [diff] [review] > Patch v1 > > Ok. Always zero was our old behavior, right? Yes.
Assignee | ||
Comment 23•14 years ago
|
||
Comment on attachment 488902 [details] [diff] [review] Part 1 - Make select.size returns 0 by default Actually, I realized there would be a regression with this patch: <select multiple> should show 4 values by default instead of all values with this patch applied. I will attach another patch to fix that.
Attachment #488902 -
Attachment description: Patch v1 → Part 1 - Make select.size returns 0 by default
Assignee | ||
Comment 24•14 years ago
|
||
Attachment #488976 -
Flags: review?(bzbarsky)
![]() |
||
Comment 25•14 years ago
|
||
Comment on attachment 488976 [details] [diff] [review] Part 2 - <select multiple> should show 4 rows by default Wait, doesn't this make this testcase: data:text/html,<select multiple><option>a<option>b<option>c<option>d<option>e</select> render differently on trunk from webkit and Fx3.6? I guess it was already different, but that was a bug, not a feature, I'd think. It looks like the new behavior matches Opera, though; what does IE do?
Assignee | ||
Comment 26•14 years ago
|
||
(In reply to comment #25) > Comment on attachment 488976 [details] [diff] [review] > Part 2 - <select multiple> should show 4 rows by default > > Wait, doesn't this make this testcase: > > data:text/html,<select > multiple><option>a<option>b<option>c<option>d<option>e</select> > > render differently on trunk from webkit and Fx3.6? I guess it was already > different, but that was a bug, not a feature, I'd think. It wasn't a bug. It's what the specs require: "The size attribute gives the number of options to show to the user. The size attribute, if specified, must have a value that is a valid non-negative integer greater than zero. If the multiple attribute is present, then the size attribute's default value is 4. If the multiple attribute is absent, then the size attribute's default value is 1. The display size of a select element is the result of applying the rules for parsing non-negative integers to the value of element's size attribute, if it has one and parsing it is successful. If applying those rules to the attribute's value is not successful, or if the size attribute is absent, the element's display size is the default value of the attribute." (this has nothing to do with what the IDL attribute returns) > It looks like the new behavior matches Opera, though; what does IE do? I should install IE on wine but didn't so I can't test on IE for the moment.
Assignee | ||
Comment 27•14 years ago
|
||
(In reply to comment #26) > > It looks like the new behavior matches Opera, though; what does IE do? > > I should install IE on wine but didn't so I can't test on IE for the moment. IE 6 shows 4 rows.
![]() |
||
Comment 28•14 years ago
|
||
Yes, I know what the spec _says_. I was just questioning _why_ it says it. Sounds like it went with the IE/Opera behavior over ours, ok. So IE shows 4 rows no matter what's going on with optgroups?
Assignee | ||
Comment 29•14 years ago
|
||
(In reply to comment #28) > Yes, I know what the spec _says_. I was just questioning _why_ it says it. > > Sounds like it went with the IE/Opera behavior over ours, ok. > > So IE shows 4 rows no matter what's going on with optgroups? It shows 4 lines no matter what's going on with optgroups. <select multiple> <optgroup label='foo'> <option>opt 1</option> <option>opt 2</option> </optgroup> <option>opt 3</option> <option>opt 4</option> </select> Will show: foo opt 1 opt 2 opt 3 Same behavior with IE, Opera and Minefield.
![]() |
||
Comment 30•14 years ago
|
||
What will it show if optgroup and option have different sizes?
Assignee | ||
Comment 31•14 years ago
|
||
(In reply to comment #30) > What will it show if optgroup and option have different sizes? You mean if we style the font-size of optgroup and option for example? It seems like other browsers (IE and Opera at least) don't accept that and our behavior is already buggy with size and optgroup/option with different size. It looks like we are using the biggest row size to compute the global size.
![]() |
||
Comment 32•14 years ago
|
||
Yes, styling the font-size would do what I was asking about. We purposefully use the bigger size; the idea is to show at least N rows in their entirety. I guess if the other browsers don't support that, we can just ignore that issue for now...
![]() |
||
Comment 33•14 years ago
|
||
Comment on attachment 488976 [details] [diff] [review] Part 2 - <select multiple> should show 4 rows by default r=me, though this lowest-common-denominator ugliness makes me sad. :(
Attachment #488976 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 34•14 years ago
|
||
Pushed: http://hg.mozilla.org/mozilla-central/rev/73fce7283157 http://hg.mozilla.org/mozilla-central/rev/5c43321021b1 We need the dev doc to be updated (and specify that 0 is the default value). (In reply to comment #33) > r=me, though this lowest-common-denominator ugliness makes me sad. :( Me too but I guess we don't really have the choice, do we?
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Keywords: dev-doc-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Comment 35•14 years ago
|
||
I have updated: https://developer.mozilla.org/en/HTML/Element/select It now says we return 0 by default, and has a note explaining why.
Assignee | ||
Comment 36•14 years ago
|
||
"The default value is 0 unless the multiple attribute is defined, in which case it is 4." Should be: "The default value is 0."
Comment 37•14 years ago
|
||
Fixed.
Updated•14 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
Comment 38•14 years ago
|
||
Guys, Please download latest fixed from the link below. http://www.marghoobsuleman.com/jquery-image-dropdown Thank you all and sorry for inconvenience. Regards, Marghoob Suleman
You need to log in
before you can comment on or make changes to this bug.
Description
•