Closed
Bug 830260
Opened 12 years ago
Closed 10 years ago
Menu items remain displayed as selected after moving mouse over them on http://lcfvs.com/index.xhtml
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: manuela.muntean, Assigned: bzbarsky)
References
()
Details
(Keywords: regression)
Attachments
(1 file)
(deleted),
patch
|
heycam
:
review+
|
Details | Diff | Splinter Review |
Latest Nightly, User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130113 Firefox/21.0, Build ID: 20130113031019
STR:
1. Open Nightly and go to http://lcfvs.com/index.xhtml
2. Press the red tab on the left (this will start the menu animation)
3. Start moving the mouse up and down, on the menu items.
Actual Results:
Menu items remain displayed as selected after moving mouse over them.
Expected Results:
Only the current selected item with the mouse should be displayed as selected (not all menu items).
Comment 1•12 years ago
|
||
Works fine in Firefox 16, but I can see the bug in nightly. We should find out what regressed this.
Keywords: regressionwindow-wanted
Reporter | ||
Comment 2•12 years ago
|
||
I can also reproduce this issue in Firefox 18 (Build ID: 20130104151925)
Reporter | ||
Updated•12 years ago
|
Version: unspecified → Trunk
Updated•12 years ago
|
Comment 3•12 years ago
|
||
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/f077de66e52d
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120824195358
Bad:
http://hg.mozilla.org/mozilla-central/rev/e874475efe15
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120825084301
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f077de66e52d&tochange=e874475efe15
Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/67ff83142ba5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120824174558
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/89e186d2a171
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120824181259
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=67ff83142ba5&tochange=89e186d2a171
In local build
Last Good: 75f3cd90e743
First Bad: c526d9dfb684
Triggered by:
c526d9dfb684 Boris Zbarsky — Bug 753517 part 4. Set up auto-generation of CSS2Properties.webidl from nsCSSPropList.h and enable Paris bindings for CSSStyleDeclaration and CSS2Properties. r=khuey,peterv,dbaron
Blocks: 753517
status-firefox18:
--- → affected
status-firefox-esr17:
--- → affected
tracking-firefox18:
--- → ?
tracking-firefox-esr17:
--- → ?
Component: Layout: View Rendering → DOM
Keywords: regressionwindow-wanted → regression
Assignee | ||
Comment 4•12 years ago
|
||
[09:31:25.043] Expected color but found 'null'. Error in parsing value for 'fill'. Declaration dropped. @ http://lcfvs.com/index.xhtml
This does not appear in fx16.
The relevant site code is on line 60 of http://lcfvs.com/stretchMenu.js and looks like this:
['onmouseout',function(){
document.getNodes(this.id.replace(/^(menu)/,
'menu_svg')).style.setProperty('fill',null,null);
Note the second argument to setProperty. The old binding used to convert null to "", while the new ones convert it to "null" per spec. A simple testcase for this behavior:
<!DOCTYPE html>
<div id="x" style="color: red">X</div>
<div id="y" style="color: red">Y</div>
<script>
document.querySelector("#x").style.setProperty("color", "null");
document.querySelector("#y").style.setProperty("color", null);
</script>
shows that WebKit seems to have the same behavior as Gecko used to, while Opera has Gecko's new behavior.
Assignee | ||
Comment 5•12 years ago
|
||
IE9 has weird behavior where setting to "null" removes the style but setting to null doesn't.
On the other hand, IE9 works on this site. Why, exactly?
In any case, the options here are to change the spec to make this argument turn null into "" instead of "null" or to evangelize the site to fix their code to follow the spec. David, thoughts?
Comment 6•12 years ago
|
||
Given that FF17 is affected, and this isn't a critical regression, we'd accept a low risk uplift if found.
Assignee | ||
Updated•12 years ago
|
Flags: needinfo?(dbaron)
Comment 7•12 years ago
|
||
No strong opinions here, as far as whether null should convert to "" or "null". I'd like to avoid IE's special-behavior-for-"null", though (if I correctly understood what you said about IE).
Does WebIDL already have an annotation to say that null gets turned into the empty string? (Are there other things that have that behavior.)
Flags: needinfo?(dbaron)
Assignee | ||
Comment 8•12 years ago
|
||
> Does WebIDL already have an annotation to say that null gets turned into the empty
> string? (Are there other things that have that behavior.)
Yes on both counts.
(In reply to Boris Zbarsky (:bz) from comment #4)
> [09:31:25.043] Expected color but found 'null'. Error in parsing value for
> 'fill'. Declaration dropped. @ http://lcfvs.com/index.xhtml
>
> This does not appear in fx16.
>
> The relevant site code is on line 60 of http://lcfvs.com/stretchMenu.js and
> looks like this:
>
> ['onmouseout',function(){
> document.getNodes(this.id.replace(/^(menu)/,
>
> 'menu_svg')).style.setProperty('fill',null,null);
>
> Note the second argument to setProperty. The old binding used to convert
> null to "", while the new ones convert it to "null" per spec. A simple
> testcase for this behavior:
>
> <!DOCTYPE html>
> <div id="x" style="color: red">X</div>
> <div id="y" style="color: red">Y</div>
> <script>
> document.querySelector("#x").style.setProperty("color", "null");
> document.querySelector("#y").style.setProperty("color", null);
> </script>
>
> shows that WebKit seems to have the same behavior as Gecko used to, while
> Opera has Gecko's new behavior.
Thanks a lot, fixed. ;)
Assignee | ||
Comment 10•10 years ago
|
||
OK, this apparently got changed in the spec too. Spec now says:
void setProperty(DOMString property, [TreatNullAs=EmptyString] DOMString value, [TreatNullAs=EmptyString] optional DOMString priority = "");
Assignee | ||
Comment 11•10 years ago
|
||
Attachment #8529308 -
Flags: review?(cam)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment 12•10 years ago
|
||
Comment on attachment 8529308 [details] [diff] [review]
Passing null as the value to CSSStyleDeclaration.setProperty should remove the property
Review of attachment 8529308 [details] [diff] [review]:
-----------------------------------------------------------------
::: layout/style/test/test_setPropertyWithNull.html
@@ +23,5 @@
> + ise(div.style.color, "",
> + 'Passing null as a color to setProperty should remove the property');
> +
> + div.style.color = "green";
> + ise(div.style.color, "green", 'Assigning "green" as a color should not parse');
should parse
Attachment #8529308 -
Flags: review?(cam) → review+
Assignee | ||
Comment 13•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2791f426d6f and https://hg.mozilla.org/integration/mozilla-inbound/rev/120949d95429 to actually address the review comment. :(
Target Milestone: --- → mozilla37
Comment 14•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f2791f426d6f
https://hg.mozilla.org/mozilla-central/rev/120949d95429
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•