third_party/WebKit/LayoutTests/plugins/object-onload-placeholder.html - Issue 2733083004: Emit error events if the loading of an object element failed

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: third_party/WebKit/LayoutTests/plugins/object-onload-placeholder.html

Issue 2733083004: Emit error events if the loading of an object element failed (Closed)

Patch Set: Emit error events if the loading of an object element failed Created 3 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « third_party/WebKit/LayoutTests/plugins/object-onerror-placeholder.html ('k') | third_party/WebKit/Source/core/html/HTMLObjectElement.cpp » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <html>

	2 <script src="../resources/testharness.js"></script>

	3 <script src="../resources/testharnessreport.js"></script>

	4 <body>

	5 <object type="image/gif" data="resources/apple.gif" onload="loadHandler()"

	6 onerror="errorHandler()"></object>

	7 <script>

	8 var testobject = async_test("crbug.com/445557:error events aren't dispatched" +

	9 " for <object> elements");

	10 function loadHandler() {

	11 testobject.done();

	12 }

	13 function errorHandler() {

	14 setTimeout(function () {
	Bernhard Bauer 2017/04/18 10:08:02 I think the timeout is in the wrong place here. Yo I think the timeout is in the wrong place here. You want the timeout in the load handler to delay passing the test (in case the error event ends up on the message loop right after it). Also, nit: Using arrow syntax (https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Functions/Arro...) for the function would look a bit nicer. George Joseph 2017/04/18 14:18:45 Done. Show quoted text On 2017/04/18 10:08:02, Bernhard Bauer wrote: > I think the timeout is in the wrong place here. You want the timeout in the load > handler to delay passing the test (in case the error event ends up on the > message loop right after it). > > Also, nit: Using arrow syntax > (https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Functions/Arro...) > for the function would look a bit nicer. Done.
	15 testobject.step(function() {

	16 assert_true(false);
	Bernhard Bauer 2017/04/18 10:08:02 I would use assert_unreached(<message>), as that c I would use assert_unreached(<message>), as that conveys the intent more clearly than asserting that true is false :) and it lets you specify a nice error message. George Joseph 2017/04/18 14:18:45 Done. Show quoted text On 2017/04/18 10:08:02, Bernhard Bauer wrote: > I would use assert_unreached(<message>), as that conveys the intent more clearly > than asserting that true is false :) and it lets you specify a nice error > message. Done.
	17 });

	18 testobject.done();
	Bernhard Bauer 2017/04/18 10:08:02 This call is also unnecessary. This call is also unnecessary. George Joseph 2017/04/18 14:18:45 Done. Show quoted text On 2017/04/18 10:08:02, Bernhard Bauer wrote: > This call is also unnecessary. Done.
	19 },0);
	Bernhard Bauer 2017/04/18 10:08:02 Nit: space after comma. Nit: space after comma. George Joseph 2017/04/18 14:18:45 Done. Show quoted text On 2017/04/18 10:08:02, Bernhard Bauer wrote: > Nit: space after comma. Done.
	20 };

	21 </script>

	22 </body>

	23 </html>

OLD	NEW