Bug 20038 – REGRESSION (r35151): Can't post comments on flickr.com

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Matias Canelson]

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.

[Dave Bullock / eecue]

I am having this same problem, I just downloaded the latest release of WK and the problem still exists.

[Robert Brook]

I'm seeing this behaviour on WebKit "3.1.2 (5525.20.1)". Should this bug have the keyword "YahooBug"?

[Mark Rowe (bdash)]

<rdar://problem/6092270>

[Deirdre Saoirse Moen]

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]

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]

works in r35138
broken in r35153

[Adele Peterson]

Regressed with http://trac.webkit.org/changeset/35151

[Adele Peterson]

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)]

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)]

I'm going to look into this.

[Dan C]

(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]

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

[David Kilzer (:ddkilzer)]

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)]

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)]

(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)]

(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]

(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)]

(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)]

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 on attachment 22662 [details]
Patch v1

Coming soon:  automated layout test.

[David Kilzer (:ddkilzer)]

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 on attachment 22669 [details]
Patch v2

Looks good to me!

[David Kilzer (:ddkilzer)]

$ 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)]

(In reply to comment #7)
> Regressed with http://trac.webkit.org/changeset/35151

See Bug 13067.