Closed
Bug 985988
Opened 11 years ago
Closed 11 years ago
event.defaultPrevented returns different value from other browsers (IE, Chrome)
Categories
(Core :: DOM: Events, defect)
Tracking
()
VERIFIED
FIXED
mozilla31
People
(Reporter: alice0775, Assigned: smaug)
References
Details
(Keywords: regression, site-compat, Whiteboard: [parity-Chrome][parity-ie])
Attachments
(4 files, 2 obsolete files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
masayuki
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
When I test Bug 985463, I fond the following discrepancy between Firefox and other browser.
<a id="a"
onclick="return false;"
href="https://bugzilla.mozilla.org/">link</a>
<script>
document.getElementById("a").addEventListener("click", log, false);
function log(event) {
alert(event.defaultPrevented);
}
</script>
Steps To Reproduce:
1. open testcase
2. Click "link"
Actual Results:
Firefox28 and later returns false.
IE11 and Crome33 return true.
Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/06b3a7aea2c0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131208154802
Bad:
http://hg.mozilla.org/mozilla-central/rev/551efcc4de6f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131208185601
Puahlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=06b3a7aea2c0&tochange=551efcc4de6f
Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/046cf0c4f858
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131207204100
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/2aaff66026ba
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 ID:20131208075200
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=046cf0c4f858&tochange=2aaff66026ba
Regressed by:
Bug 930374 - Event.defaultPrevented shouldn't become true if preventDefault() was called by our internal handler for default action
Assignee | ||
Comment 1•11 years ago
|
||
Uh, need to fix nsJSEventListener::HandleEvent
Assignee | ||
Comment 2•11 years ago
|
||
Masayuki, want to look at this?
Thanks Alice!
Flags: needinfo?(masayuki)
Assignee | ||
Comment 3•11 years ago
|
||
Or patch coming. Need to write some tests
Assignee: nobody → bugs
Flags: needinfo?(masayuki)
Assignee | ||
Comment 4•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=4b0e5f0d0310
beta patch will need some renaming, since there we have still nsDOMEvent
Attachment #8394404 -
Flags: review?(masayuki)
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #8394404 -
Attachment is obsolete: true
Attachment #8394404 -
Flags: review?(masayuki)
Attachment #8394425 -
Flags: review?(masayuki)
Assignee | ||
Comment 6•11 years ago
|
||
Crossing fingers this more conventional approach compiles
https://tbpl.mozilla.org/?tree=Try&rev=fe69147db0df
Attachment #8394425 -
Attachment is obsolete: true
Attachment #8394425 -
Flags: review?(masayuki)
Attachment #8394440 -
Flags: review?(masayuki)
Comment 7•11 years ago
|
||
I'm sorry. Today is national holiday of Japan. And I'm going to go out soon. When I get back home and I have much time then, I'll check this.
Updated•11 years ago
|
Comment 8•11 years ago
|
||
Comment on attachment 8394440 [details] [diff] [review]
v3
Although, I don't understand well the computation code of isChromeHandler, but you're the expert around here. So, I trust your code.
The other concern is, I'm not sure if GetHandler() and GetHandler().Ptr() are always non-null when HandleEvent() is called. If it's no problem, r=masayuki.
Thank you for your work!
Attachment #8394440 -
Flags: review?(masayuki) → review+
Assignee | ||
Comment 9•11 years ago
|
||
HasEventHandler() checks the Ptr isn't null.
Assignee | ||
Comment 10•11 years ago
|
||
Assignee | ||
Comment 11•11 years ago
|
||
Assignee | ||
Comment 12•11 years ago
|
||
Comment on attachment 8394440 [details] [diff] [review]
v3
[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 930374
User impact if declined: bug 985463
Testing completed (on m-c, etc.): landed to m-i
Risk to taking this patch (and alternatives if risky): Shouldn't be too risky
String or IDL/UUID changes made by this patch: NA
Attachment #8394440 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 13•11 years ago
|
||
[Approval Request Comment]
See for request for aurora
Attachment #8394938 -
Flags: approval-mozilla-beta?
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Updated•11 years ago
|
Attachment #8394938 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•11 years ago
|
Attachment #8394440 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 15•11 years ago
|
||
verified fixed on nightly 31.0a1 20140322030204.
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
QA Contact: ananuti
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 16•11 years ago
|
||
Comment 17•11 years ago
|
||
verified on aurora 30.0a2 20140324150430
Comment 18•11 years ago
|
||
I still can reproduce on 29.0-beta2.
buildID: 20140324101726
changeset fceb90d30601
Clicking `link` on attachment 8394205 [details] returns false.
Flags: needinfo?(bugs)
Comment 19•11 years ago
|
||
oops, firefox 29 beta 2 came off changeset 662d9090988b which was prior to this landing (see hg graph) :(
Flags: needinfo?(bugs)
Comment 20•11 years ago
|
||
verified on current tinderbox mozilla-beta changeset 33d1476deae6.
Keywords: verifyme
Updated•11 years ago
|
Keywords: site-compat
You need to log in
before you can comment on or make changes to this bug.
Description
•