304882 - bookmarklet fails (accessing location.href after document.write())

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050816
Firefox/1.0+ ID:2005081613

1. create bookmarklet (attachment follows)
2. open this bug and press the bookmarklet

expected: the textarea should be filled with text

result JSC error:
Error: uncaught exception: [Exception... "Component returned failure code:
0x80004003 (NS_ERROR_INVALID_POINTER) [nsIDOMLocation.href]"  nsresult:
"0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS frame ::
javascript:(function() { var title = document.title; function
htmlEscape(s){return s;}; var
OS=document.getElementById(%22op_sys%22).value.substring(0,3) ;var
PR=document.getElementById(%22product%22).value ;var
comp=document.getElementById(%22component%22).value;if(comp==%22XSLT%22||comp==%22MathML%22||comp==%22XForms%22||comp==%22SVG%22||comp==%22Layout:
Canvas%22||comp==%22Editor%22||comp==%22Build Config%22)
PR=PR+%22:%22+comp;;document.write(%22<form name=f><textarea name=ta cols=120
rows=5></textarea></form>%22); document.close(); document.f.ta.value =
%22[*][url=%22 + location.href + %22]#%22 + /\d+/(title)[0] + %22 [/url][%22 +
PR +%22]-%22+htmlEscape(/-.*/(title)[0].slice(2)) +%22 [%22+OS +%22]%22+ %22%22;
document.f.ta.select(); })() :: anonymous :: line 1"  data: no]

regressionwindow:
http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?treeid=default&module=AviarySuiteBranchTinderbox&branch=MOZILLA_1_8_BRANCH&branchtype=match&filetype=match&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-08-16+11%3A47%3A00&maxdate=2005-08-16+13%3A29%3A00&cvsroot=%2Fcvsroot

Comment 1

[Peter van der Woude [:Peter6]]

Attachment: bookmarklet as textfile

Comment 2

[Jesse Ruderman]

I'm guessing this is jst's.

Comment 3

[Dave Townsend [:mossop]]

The trunk nightly has this regression as well.

Comment 4

[Jesse Ruderman]

A simpler bookmarklet that fails now on the branch but didn't fail with this
morning's branch nightly:

javascript:document.write(); alert(location.href);

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: the patch + extra changes

This is fallout from my checkin to the branch earlier today (bug 303267). This
patch switches us to actually doing security checks for accesses to the
location object instead of doing the null-out-docshell dance.

This patch conflicts with my patch for bug 304764, so I'll attach a new one
when I can extract the relevant changes.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 192881 [details] [diff] [review]
the patch + extra changes

+  // object. Parent it to the inner window so that access checks do the Right

Note to self: Parent it to the *outer* window

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: real patch

This patch should apply directly on top of the patch for bug 304764 (i.e.,
apply that one first, then apply this one).

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 192902 [details] [diff] [review]
real patch

Looks good. Make the changes we talked about (nulling out the docshell in the
window state holders' dtor and asserting in PreCreate() if we don't have a
docshell) and I'll r=...

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: null out the docshell when the docshell goes away

This has the extra listener manager changes in it, but I promise those aren't
part of this bug :-).

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 193008 [details] [diff] [review]
null out the docshell when the docshell goes away

r=jst

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 193008 [details] [diff] [review]
null out the docshell when the docshell goes away

... and sr=jst too!

Comment 12

[Johnny Stenback (:jst)]

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

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Checked into trunk.

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 193008 [details] [diff] [review]
null out the docshell when the docshell goes away

This fixes window.location to work closer to the way it did before the
splitwinodw landing.

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

I had to back this out because it was turning Txul tests orange.

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into the trunk, though the tree went red before any of the
tinderboxen that went orange the last time had a chance to go green. I'm marking
this FIXED, optimistically. I'll check in on the branch tomorrow.

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

And checked into MOZILLA_1_8_BRANCH. Hopefully it'll have more luck there!

Comment 18

[Boris Zbarsky [:bzbarsky]]

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

Comment 19

[Boris Zbarsky [:bzbarsky]]

This probably caused bug 338288