110521 – Move setAutofilled from TestRunner to WebCore

RESOLVED FIXED 110521

Move setAutofilled from TestRunner to WebCore

https://bugs.webkit.org/show_bug.cgi?id=110521

Summary Move setAutofilled from TestRunner to WebCore

Jason Anderssen

Reported 2013-02-21 15:24:20 PST

Move setAutofilled from TestRunner to WebCore

Attachments
To move the setAutofilled from TestRunner to WebCore (52.60 KB, patch) 2013-02-21 21:11 PST, Jason Anderssen	no flags	Details Formatted Diff Diff
Patch (27.10 KB, patch) 2013-02-22 17:55 PST, Jason Anderssen	no flags	Details Formatted Diff Diff
Patch (27.86 KB, patch) 2013-02-22 18:45 PST, Jason Anderssen	no flags	Details Formatted Diff Diff
Patch (27.74 KB, patch) 2013-02-22 19:05 PST, Jason Anderssen	no flags	Details Formatted Diff Diff
Patch for landing (25.86 KB, patch) 2013-02-22 22:38 PST, Benjamin Poulain	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Jason Anderssen

Comment 1 2013-02-21 21:11:04 PST

Created attachment 189674 [details] To move the setAutofilled from TestRunner to WebCore

Benjamin Poulain

Comment 2 2013-02-21 23:11:04 PST

Comment on attachment 189674 [details] To move the setAutofilled from TestRunner to WebCore View in context: https://bugs.webkit.org/attachment.cgi?id=189674&action=review Congrats on making your first WebKit patch. It looks like many files that should be left unmodified have been modified. I suspect there is something up with the line endings. You should revert those files and find what is modifying them on your system. > Tools/WinLauncher/WinLauncher.vcxproj/WinLauncherResource.h:-2 > -//{{NO_DEPENDENCIES}} > -// Microsoft Visual C++ generated include file. This file should not be modified. > Tools/WinLauncher/WinLauncherLauncherResource.h:-2 > -//{{NO_DEPENDENCIES}} > -// Microsoft Visual C++ generated include file. Ditto. > Source/WebCore/testing/Internals.cpp:819 > + if (!element->hasTagName(inputTag)) { > + ec = INVALID_ACCESS_ERR; > + return; > + } You don't need this part of the check. Node::toInputElement is polymorphic and will return null for anything that is not an input element. > Source/WebCore/testing/Internals.cpp:825 > + inputElement->setAutofilled( enabled ); To follow the WebKit style, you should not have spaces around the argument. > Source/WebCore/testing/Internals.h:212 > + void setAutofilled(Element* element, bool enabled, ExceptionCode&); > + I would put this with the other functions acting on <input>: setSuggestedValue, setEditingValue, etc. The first parameter would probably better be named "inputElement". > Source/WebCore/testing/Internals.idl:255 > + > + void setAutofilled(in Element scope, in boolean enabled) raises(DOMException); Ditto. "scope" is probably worse. Keeping the style consistent, you need a space between raises and the exception type. > Source/WebCore/ChangeLog:6 > + Move setAutofilled from TestRunner to WebCore > + https://bugs.webkit.org/show_bug.cgi?id=110521 > + > + Reviewed by NOBODY (OOPS!). In this changelog, you should put a word of description about the reason of the change. See other patches to get a feeling of what kind of information is important. > Source/WebCore/ChangeLog:8 > + No new tests (OOPS!). This line should always be removed. Usually, you replace it by the list of tests covering the change. In this case, there is no additional test, we are working on test, so you can just delete it. > Source/ThirdParty/gtest/codegear/gtest_all.cc:-2 > -// Copyright 2009, Google Inc. > -// All rights reserved. This file should not be modified. > Source/ThirdParty/gtest/codegear/gtest_link.cc:-2 > -// Copyright 2009, Google Inc. > -// All rights reserved. Ditto. > Source/ThirdParty/ChangeLog:2 > +2013-02-21 Jason Anderssen <janderssen@exactal.com> > + You can clear this ChangeLog once you revert the file to the original. > LayoutTests/storage/indexeddb/set_version_blocked.html:-2 > -<html> > -<head> This should not be modified. > LayoutTests/storage/indexeddb/transaction-read-only.html:-2 > -<html> > -<head> This should not be modified. > LayoutTests/css2.1/20110323/support/eof-green.css:-2 > -/* eof-green.css */ > -div This should not be modified. > LayoutTests/css2.1/20110323/support/at-import-001.css:-2 > -div > -{ This should not be modified. > LayoutTests/css2.1/20110323/support/at-import-002.css:-2 > -div > -{ This should not be modified. > LayoutTests/css2.1/20110323/support/at-import-004.css:-2 > -div > -{ This should not be modified. > LayoutTests/css2.1/20110323/support/at-import-005.css:-2 > -div > -{ This should not be modified. > LayoutTests/css2.1/20110323/support/at-import-006.css:-2 > -div > -{ This should not be modified. > LayoutTests/css2.1/20110323/support/at-import-007.css:-2 > -div > -{ This should not be modified. > LayoutTests/inspector/storage-panel-dom-storage-update.html:-2 > -<html> > -<head> This should not be modified. > LayoutTests/compositing/repaint/shrink-layer.html:-2 > -<html> > -<head> This should not be modified.

Jason Anderssen

Comment 3 2013-02-22 17:55:21 PST

Created attachment 189878 [details] Patch

Benjamin Poulain

Comment 4 2013-02-22 18:33:36 PST

Comment on attachment 189878 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189878&action=review > Source/WebCore/ChangeLog:8 > + No new tests (OOPS!). You should remove this line from the ChangeLog. > Source/WebCore/testing/Internals.cpp:825 > + if (!element->hasTagName(inputTag)) { > + ec = INVALID_ACCESS_ERR; > + return; > + } You should remove this. > Source/WebCore/testing/Internals.idl:257 > + void setAutofilled(in Element scope, in boolean enabled) raises(DOMException); scope -> inputElement

Benjamin Poulain

Comment 5 2013-02-22 18:38:24 PST

Comment on attachment 189878 [details] Patch r- based on the comments + the symbol for HTMLInputElement::setAutofilled is missing on GTK.

Jason Anderssen

Comment 6 2013-02-22 18:45:24 PST

Created attachment 189893 [details] Patch

Jason Anderssen

Comment 7 2013-02-22 19:05:38 PST

Created attachment 189895 [details] Patch

Benjamin Poulain

Comment 8 2013-02-22 22:38:17 PST

Created attachment 189908 [details] Patch for landing

WebKit Review Bot

Comment 9 2013-02-23 00:16:26 PST

Comment on attachment 189908 [details] Patch for landing Clearing flags on attachment: 189908 Committed r143837: <http://trac.webkit.org/changeset/143837>

WebKit Review Bot

Comment 10 2013-02-23 00:16:31 PST

All reviewed patches have been landed. Closing bug.

WebKit Review Bot

Comment 11 2013-02-24 11:29:14 PST

Re-opened since this is blocked by bug 110713

Daniel Bates

Comment 12 2017-12-14 12:30:29 PST

*** Bug 88239 has been marked as a duplicate of this bug. ***

Radar WebKit Bug Importer

Comment 13 2017-12-14 12:33:56 PST

<rdar://problem/36055133>

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware All

OS All

Product WebKit

Component Tools / Tests

Assignee

Jason Anderssen

Reported

2013-02-21 15:24 PST

Modified

2017-12-14 12:33 PST History

CC List

14 users Show

URL

Keywords InRadar

Duplicates (1)

88239 View as bug list

Depends on

110713

Blocks

Dependencies

tree graph