Bug 20038

Summary: REGRESSION (r35151): Can't post comments on flickr.com
Product: WebKit Reporter: Matias Canelson <canelson>
Component: FormsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Major CC: beidson, dantearmok, ddkilzer, desamo, henrycavillones, hyatt
Priority: P2 Keywords: InRadar, Regression, YahooBug
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
URL: http://www.flickr.com/
Attachments:
Description Flags
Patch v1
ddkilzer: review-
Patch v2 eric: review+

Matias Canelson
Reported 2008-07-14 21:51:50 PDT
While using flickr website, I tried to add a comment to a photo and it can't be done. After pressing the POST COMMENT button nothing happens. I tried using Safari 3.1 and Firefox on the same computer and it works fine.
Attachments
Patch v1 (6.40 KB, patch)
2008-08-05 15:14 PDT, David Kilzer (:ddkilzer)
ddkilzer: review-
Patch v2 (7.73 KB, patch)
2008-08-05 17:44 PDT, David Kilzer (:ddkilzer)
eric: review+
Dave Bullock / eecue
Comment 1 2008-07-17 07:06:59 PDT
I am having this same problem, I just downloaded the latest release of WK and the problem still exists.
Robert Brook
Comment 2 2008-07-19 00:31:10 PDT
I'm seeing this behaviour on WebKit "3.1.2 (5525.20.1)". Should this bug have the keyword "YahooBug"?
Mark Rowe (bdash)
Comment 3 2008-07-21 21:16:43 PDT
Deirdre Saoirse Moen
Comment 4 2008-07-23 17:11:58 PDT
Just to be clear, did you mean this didn't work with the released 3.1.2, or with a WebKit build running against 3.1.2? I just tried it on a stock 10.5.4/3.1.2 machine and had couldn't reproduce.
Matias Canelson
Comment 5 2008-07-23 17:31:22 PDT
Webkit v35289 downloaded on July 23rd doesn't work. Official Safari v3.1.2 (5525.20.1) does work. I'm on a Macbook CoreDuo 2Ghz with MAC OS X 10.5.4 with every Apple software up to date.
Adele Peterson
Comment 6 2008-07-24 00:58:54 PDT
works in r35138 broken in r35153
Adele Peterson
Comment 7 2008-07-24 10:00:48 PDT
Adele Peterson
Comment 8 2008-07-24 12:17:28 PDT
This issues occurs when your original url as a trailing slash. Flickr adds a #preview to the url before doing another load to add the comment. The new code added in r 35151 prevents reloading in that case where #preview is added. I don't know how we can tell the difference between the case where the site adds a hash and intends to do more processing on a reload, and when a user adds a hash to jump somewhere in the page.
David Kilzer (:ddkilzer)
Comment 9 2008-07-24 16:38:57 PDT
Ugh. Does this work on Firefox 2.0/3.0? Perhaps we're missing a "userInitiatedLoad" flag to know when the change was done by JavaScript versus the user.
David Kilzer (:ddkilzer)
Comment 10 2008-07-25 08:48:42 PDT
I'm going to look into this.
Dan C
Comment 11 2008-07-27 13:42:19 PDT
(In reply to comment #9) > Does this work on Firefox 2.0/3.0? Seeing the same problem in Adium's trac system and others - they all add #preview etc. This works from current Safari 3.1.2, and Firefox 2 and 3.0.1 but not from the latest (27 July) WebKit nightly. I first noticed this problem in r35249 (built on 20 July 2008), fwiw.
Pierre-Luc Beaudoin
Comment 12 2008-07-28 08:04:22 PDT
*** Bug 20195 has been marked as a duplicate of this bug. ***
David Kilzer (:ddkilzer)
Comment 13 2008-08-03 06:21:52 PDT
Unfortunately r35377 just refactored many of the methods. Not sure if this is going to help or hurt. http://trac.webkit.org/changeset/35377
David Kilzer (:ddkilzer)
Comment 14 2008-08-03 11:52:23 PDT
I have a fix! The problem is that FrameLoader::m_formAboutToBeSubmitted is never set on a form that doesn't have any "text fields", which causes a FormState object not be created, which causes isFormSubmission not to be set to true, which means that shouldScrollToAnchor() returns true instead of false. Next step is to run layout tests and to write a new layout test.
David Kilzer (:ddkilzer)
Comment 15 2008-08-03 11:56:08 PDT
(In reply to comment #11) > Seeing the same problem in Adium's trac system and others - they all add > #preview etc. This works from current Safari 3.1.2, and Firefox 2 and 3.0.1 > but not from the latest (27 July) WebKit nightly. Dan, what are the steps to reproduce this issue?
David Kilzer (:ddkilzer)
Comment 16 2008-08-03 12:05:57 PDT
(In reply to comment #15) > (In reply to comment #11) > > Seeing the same problem in Adium's trac system and others - they all add > > #preview etc. This works from current Safari 3.1.2, and Firefox 2 and 3.0.1 > > but not from the latest (27 July) WebKit nightly. > > Dan, what are the steps to reproduce this issue? I tried creating a preview of a new ticket on macosforge.net, but the preview button still works there (even though it adds "#preview" to the URL). See Comment #14.
Dan C
Comment 17 2008-08-03 12:59:53 PDT
(In reply to comment #15) > (In reply to comment #11) > > Seeing the same problem in Adium's trac system and others - they all add > > #preview etc. This works from current Safari 3.1.2, and Firefox 2 and 3.0.1 > > but not from the latest (27 July) WebKit nightly. > > Dan, what are the steps to reproduce this issue? Now using WebKit r35531, still on OS X 10.4.11. Pick a ticket, any ticket, on AdiumX's trac. eg: http://trac.adiumx.com/ticket/10582 Scroll to the end and login. Put text into the comment field. Hit "Preview" The url is rewritten to have #preview on the end, but other than that, nothing happens.
David Kilzer (:ddkilzer)
Comment 18 2008-08-03 16:43:35 PDT
(In reply to comment #17) > Pick a ticket, any ticket, on AdiumX's trac. > eg: http://trac.adiumx.com/ticket/10582 > Scroll to the end and login. > Put text into the comment field. > Hit "Preview" > The url is rewritten to have #preview on the end, but other than that, nothing > happens. Okay, this is the same issue. Thanks!
David Kilzer (:ddkilzer)
Comment 19 2008-08-05 15:14:06 PDT
Created attachment 22662 [details] Patch v1 Patch with changelog and manual test. An automated test couldn't be made since it would always succeed (used a different code path than manually clicking on the Submit button).
David Kilzer (:ddkilzer)
Comment 20 2008-08-05 15:24:01 PDT
Comment on attachment 22662 [details] Patch v1 Coming soon: automated layout test.
David Kilzer (:ddkilzer)
Comment 21 2008-08-05 17:44:57 PDT
Created attachment 22669 [details] Patch v2 Now with automated regression test. My mistake before when creating an automated test was that I tried to submit the form before the page was finished loading, preventing the bug from reproducing.
Eric Seidel (no email)
Comment 22 2008-08-05 18:36:54 PDT
Comment on attachment 22669 [details] Patch v2 Looks good to me!
David Kilzer (:ddkilzer)
Comment 23 2008-08-06 14:15:57 PDT
$ git svn dcommit Committing to http://svn.webkit.org/repository/webkit/trunk ... M LayoutTests/ChangeLog A LayoutTests/fast/forms/submit-to-url-fragment-expected.txt A LayoutTests/fast/forms/submit-to-url-fragment.html M WebCore/ChangeLog M WebCore/html/HTMLFormElement.cpp M WebCore/loader/FrameLoader.cpp M WebCore/loader/FrameLoader.h Committed r35611
David Kilzer (:ddkilzer)
Comment 24 2009-01-04 08:44:11 PST
(In reply to comment #7) > Regressed with http://trac.webkit.org/changeset/35151 See Bug 13067.
Note You need to log in before you can comment on or make changes to this bug.