Closed
Bug 985988
Opened 9 years ago
Closed 9 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)
242 bytes,
text/html
|
Details | |
8.65 KB,
patch
|
masayuki
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
8.66 KB,
patch
|
Details | Diff | Splinter Review | |
8.63 KB,
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•9 years ago
|
||
Uh, need to fix nsJSEventListener::HandleEvent
Assignee | ||
Comment 2•9 years ago
|
||
Masayuki, want to look at this? Thanks Alice!
Flags: needinfo?(masayuki)
Assignee | ||
Comment 3•9 years ago
|
||
Or patch coming. Need to write some tests
Assignee: nobody → bugs
Flags: needinfo?(masayuki)
Assignee | ||
Comment 4•9 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•9 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=5fc0aa52fb33
Attachment #8394404 -
Attachment is obsolete: true
Attachment #8394404 -
Flags: review?(masayuki)
Attachment #8394425 -
Flags: review?(masayuki)
Assignee | ||
Comment 6•9 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•9 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•9 years ago
|
Comment 8•9 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•9 years ago
|
||
HasEventHandler() checks the Ptr isn't null.
Assignee | ||
Comment 10•9 years ago
|
||
Assignee | ||
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/eda84c1e8f2f
Assignee | ||
Comment 12•9 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•9 years ago
|
||
[Approval Request Comment] See for request for aurora
Attachment #8394938 -
Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/eda84c1e8f2f
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Updated•9 years ago
|
Attachment #8394938 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Updated•9 years ago
|
Attachment #8394440 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
![]() |
||
Comment 15•9 years ago
|
||
verified fixed on nightly 31.0a1 20140322030204.
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
QA Contact: ananuti
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 16•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/6a3cd07007aa https://hg.mozilla.org/releases/mozilla-beta/rev/f46bf13e9b32
![]() |
||
Comment 17•9 years ago
|
||
verified on aurora 30.0a2 20140324150430
![]() |
||
Comment 18•9 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•9 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•9 years ago
|
||
verified on current tinderbox mozilla-beta changeset 33d1476deae6.
Keywords: verifyme
Updated•8 years ago
|
Keywords: site-compat
You need to log in
before you can comment on or make changes to this bug.
Description
•