Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(382)

Issue 331323012: Revert of Throw TypeError when addEventListener or removeEventListener are called (Closed)

Created:
6 years, 6 months ago by Nils Barth (inactive)
Modified:
6 years, 6 months ago
Reviewers:
haraken, Tab Atkins
CC:
blink-reviews, arv+blink, Inactive, watchdog-blink-watchlist_google.com
Visibility:
Public.

Description

Revert of Throw TypeError when addEventListener or removeEventListener are called without enough arguments (https://codereview.chromium.org/329053002/) Reason for revert: Broke Chrome OS tests: GalleryBrowserTest.CropImageOnDrive GalleryBrowserTest.CropImageOnDownloads GalleryBrowserTestInGuestMode.CropImageOnDownloads https://code.google.com/p/chromium/issues/detail?id=388071 Will add a use counter instead. Original issue's description: > Throw TypeError when addEventListener or removeEventListener are called without enough arguments > > ** Changes web-facing behavior ** > > This removes legacy behavior, per request of Tab: > it fixes some current compatibility problems which *are* causing pain, > but potentially breaks some old content. > > Current behavior (not throwing a TypeError) was for compatibility in 2012, > though unspecified cause: > WebKit bug specifically special-casing this in JavaScriptCore bindings is: > [JSC] Regression: addEventListener() and removeEventListener() raise an exception on missing args > https://bugs.webkit.org/show_bug.cgi?id=85928 > > WebKit/Safari also has legacy behavior (matches current Blink/Chrome): > "For compatibility with legacy content, the EventListener calls are generated without GenerateArgumentsCountCheck." > http://trac.webkit.org/browser/trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm#L2823 > > However, IE and Firefox *do* throw (correctly), and this discrepancy is causing > compatibility problems: > removeEventListener with only one passed parameter... > http://lists.w3.org/Archives/Public/public-script-coord/2014AprJun/0166.html > > Thus this CL aligns with spec, updating tests and removing legacy tests. > > R=haraken > BUG=353484 > TEST=fast/dom/event-target-arguments.html > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176746 TBR=haraken@chromium.org,tabatkins@chromium.org NOTREECHECKS=true NOTRY=true BUG=353484 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176800

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+175 lines, -66 lines) Patch
A LayoutTests/fast/dom/Window/window-legacy-event-listener.html View 1 chunk +17 lines, -0 lines 0 comments Download
A LayoutTests/fast/dom/Window/window-legacy-event-listener-expected.txt View 1 chunk +13 lines, -0 lines 0 comments Download
A LayoutTests/fast/dom/XMLHttpRequest-legacy-event-listener.html View 1 chunk +17 lines, -0 lines 0 comments Download
A LayoutTests/fast/dom/XMLHttpRequest-legacy-event-listener-expected.txt View 1 chunk +13 lines, -0 lines 0 comments Download
M LayoutTests/fast/dom/event-target-arguments.html View 1 chunk +30 lines, -10 lines 0 comments Download
M LayoutTests/fast/dom/event-target-arguments-expected.txt View 1 chunk +10 lines, -10 lines 0 comments Download
A LayoutTests/fast/dom/node-legacy-event-listener.html View 1 chunk +17 lines, -0 lines 0 comments Download
A LayoutTests/fast/dom/node-legacy-event-listener-expected.txt View 1 chunk +13 lines, -0 lines 0 comments Download
M LayoutTests/fast/js/function-length.html View 1 chunk +3 lines, -1 line 0 comments Download
M LayoutTests/fast/js/function-length-expected.txt View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/scripts/v8_interface.py View 2 chunks +4 lines, -5 lines 0 comments Download
M Source/bindings/scripts/v8_methods.py View 5 chunks +7 lines, -9 lines 0 comments Download
M Source/bindings/scripts/v8_types.py View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/bindings/templates/interface.cpp View 3 chunks +3 lines, -3 lines 0 comments Download
M Source/bindings/templates/methods.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/tests/idls/TestObject.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/tests/results/V8TestObject.cpp View 3 chunks +10 lines, -9 lines 0 comments Download
M Source/core/events/EventTarget.h View 1 chunk +7 lines, -2 lines 0 comments Download
M Source/core/events/EventTarget.idl View 1 chunk +6 lines, -12 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Nils Barth (inactive)
Created Revert of Throw TypeError when addEventListener or removeEventListener are called without enough arguments
6 years, 6 months ago (2014-06-24 01:49:09 UTC) #1
haraken
6 years, 6 months ago (2014-06-24 01:55:20 UTC) #2
Message was sent while issue was closed.
LGTM

Powered by Google App Engine
This is Rietveld 408576698