Closed Bug 985988 Opened 6 years ago Closed 6 years ago

event.defaultPrevented returns different value from other browsers (IE, Chrome)

Categories

(Core :: DOM: Events, defect)

28 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla31
Tracking Status
firefox28 --- wontfix
firefox29 + verified
firefox30 + verified
firefox31 + verified

People

(Reporter: alice0775, Assigned: smaug)

References

Details

(Keywords: regression, site-compat, Whiteboard: [parity-Chrome][parity-ie])

Attachments

(4 files, 2 obsolete files)

Attached file testcase
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
Uh, need to fix nsJSEventListener::HandleEvent
Masayuki, want to look at this?

Thanks Alice!
Flags: needinfo?(masayuki)
Or patch coming. Need to write some tests
Assignee: nobody → bugs
Flags: needinfo?(masayuki)
Attached patch patch + some tests (obsolete) — Splinter Review
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)
Attached patch maybe this compiles on osx (obsolete) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=5fc0aa52fb33
Attachment #8394404 - Attachment is obsolete: true
Attachment #8394404 - Flags: review?(masayuki)
Attachment #8394425 - Flags: review?(masayuki)
Attached patch v3Splinter Review
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)
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.
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+
HasEventHandler() checks the Ptr isn't null.
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?
Attached patch for beta (v3)Splinter Review
[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: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Attachment #8394938 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8394440 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
verified fixed on nightly 31.0a1 20140322030204.
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
QA Contact: ananuti
Keywords: checkin-needed
verified on aurora 30.0a2 20140324150430
I still can reproduce on 29.0-beta2.
buildID: 20140324101726
changeset fceb90d30601

Clicking `link` on attachment 8394205 [details] returns false.
Flags: needinfo?(bugs)
oops, firefox 29 beta 2 came off changeset 662d9090988b which was prior to this landing (see hg graph) :(
Flags: needinfo?(bugs)
verified on current tinderbox mozilla-beta changeset 33d1476deae6.
You need to log in before you can comment on or make changes to this bug.