Bug 12720 – Re-defining window.location.toString function keeps re-loading forever

• Component: DOM
• Status: RESOLVED FIXED

[Feng Qian]

Create a page with a statement:

window.location.toString = function() { return "www.cnn.com"; }

Open it in a new window, it keeps re-loading the page.

[David Kilzer (:ddkilzer)]

Behavior confirmed in shipping Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8N1037) and a local debug build of WebKit r19537 with the same software.

Both Firefox 2.0.0.1 and Opera 9.10 don't appear to log any errors when the test case is loaded--they just silently refuse to change the built-in function.

[David Kilzer (:ddkilzer)]

Created attachment 13098 [details]
Test case

[Feng Qian]

Created attachment 13417 [details]
patch

The bug is caused by Location::put if the property name is a function name in the LocationTable. It does not change URL, but put() will think the URL was changed and reload the file again, and into an infinite loop. The fix just returns early when it founds the property name is in the LocationTable, but not changing URL.

[Feng Qian]

Created attachment 13418 [details]
a better test with expected output

[Maciej Stachowiak]

This looks ok, but it might be better to mark the functions that should not be replaceable as read-only.

[Maciej Stachowiak]

Comment on attachment 13418 [details]
a better test with expected output

After further consideration this fix seems ok.

[Feng Qian]

(In reply to comment #5)
> This looks ok, but it might be better to mark the functions that should not be
> replaceable as read-only.
> 

How does ReadOnly attribute work? Will the call be returned before calling Location::put ?

[Mark Rowe (bdash)]

Landed in r19995.

Thanks for the patch Ian.  Can you please ensure that you use spaces rather than tabs in your ChangeLog entries, and that they are consistent in formatting with other entries.  I'm specifically thinking of new lines between description and file list, and the presence of the "Reviewed by NOBODY" line.  The latter will ensure that the patch cannot be landed unless the reviewer is added.

[Mark Rowe (bdash)]

I also just noticed that you forgot to include a changelog for the LayoutTests portion of your patch.

[Mark Rowe (bdash)]

Not to keep going on... but the expected results you provided didn't match the test.  I updated the test in r20002 to make things pass.

[Feng Qian]

(In reply to comment #8)
> Landed in r19995.
> 
> Thanks for the patch Ian.  Can you please ensure that you use spaces rather
> than tabs in your ChangeLog entries, and that they are consistent in formatting
> with other entries.  I'm specifically thinking of new lines between description
> and file list, and the presence of the "Reviewed by NOBODY" line.  The latter
> will ensure that the patch cannot be landed unless the reviewer is added.

Thanks Mike. I will try my best to remember these style rules.

>