Bug 110521

Summary: Move setAutofilled from TestRunner to WebCore
Product: WebKit Reporter: Jason Anderssen <janderssen>
Component: Tools / TestsAssignee: Jason Anderssen <janderssen>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, dbates, gyuyoung.kim, jesus, jochen, mifenton, rakuco, rego+ews, rwlbuis, tonikitoo, webkit-bug-importer, webkit.review.bot, xan.lopez, yong.li.webkit
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 110713    
Bug Blocks:    
Attachments:
Description Flags
To move the setAutofilled from TestRunner to WebCore
none
Patch
none
Patch
none
Patch
none
Patch for landing none

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
DetailsFormatted DiffDiff
Patch (27.10 KB, patch)
2013-02-22 17:55 PST, Jason Anderssen 		no flags
DetailsFormatted DiffDiff
Patch (27.86 KB, patch)
2013-02-22 18:45 PST, Jason Anderssen 		no flags
DetailsFormatted DiffDiff
Patch (27.74 KB, patch)
2013-02-22 19:05 PST, Jason Anderssen 		no flags
DetailsFormatted DiffDiff
Patch for landing (25.86 KB, patch)
2013-02-22 22:38 PST, Benjamin Poulain 		no flags
DetailsFormatted DiffDiff
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.