Summary: | [chromium] Autofill ignores forms without 'action' attribute | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jens Alfke <jens> | ||||||||||
Component: | WebKit API | Assignee: | Jens Alfke <jens> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, levin, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | All | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Attachments: |
|
Description
Jens Alfke
2010-04-22 16:59:51 PDT
Created attachment 54110 [details]
patch
Attachment 54110 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/chromium/src/WebPasswordFormData.cpp:167: One space before end of line comments [whitespace/comments] [5]
Total errors found: 1 in 1 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 54114 [details]
now, with less whitespace
OK, WebKit only wants one space before a comment. Here it is. FML.
Created attachment 54184 [details]
now with changelog entry!!
Sorry, forgot the changelog entry. I haven't touched WebKit in a few months and am getting rusty ...
Comment on attachment 54184 [details]
now with changelog entry!!
Autosave code is outside the scope of webkit historically. However is Chromium's in webkit's repo?
Either way, how do we test this?
Comment on attachment 54184 [details] now with changelog entry!! Also, your ChangeLog is wrong. You should leave the "Reviewed by" line as-is, and the scripts will automatically fill in the reviewer. it's confusing, but that's how it works. The ChangeLog shoudl also explain what testing there is or why testing is impossible. See: http://webkit.org/coding/contributing.html Created attachment 54444 [details]
updated changelog entry
OK, fixed the reviewer name glitch and added info about (the lack of) tests.
Comment on attachment 54444 [details]
updated changelog entry
Thank you. The test explanation update is helpful. One more question though:
Why fix this here, by mapping the null string to "" instead of changing completeURL()?
FrameLoader::completeURL is just a call-through wrapper for Document::completeURL), which explicitly states in a comment, "Always return a null URL when passed a null string." So this behavior appears to be intentional, and it seems likely that something would probably break, possibly in very subtle ways, if I changed it. It just happens that _in this particular case_ a missing 'action' attribute in an HTML <form> element defaults to the document's URL. Comment on attachment 54444 [details]
updated changelog entry
OK.
Comment on attachment 54444 [details] updated changelog entry Clearing flags on attachment: 54444 Committed r58382: <http://trac.webkit.org/changeset/58382> All reviewed patches have been landed. Closing bug. |