[Webkit-unassigned] [Bug 14959] No back forward entry added for pages created in javascript

[bugzilla-daemon at webkit.org]

http://bugs.webkit.org/show_bug.cgi?id=14959





------- Comment #12 from mpComplete at gmail.com  2008-01-10 12:23 PDT -------
(In reply to comment #10)
> (From update of attachment 18052 [edit])
> Patch looks great.
> 
> I have a few complaints that should be relatively simple to fix.
> 
> Many apologies for not reviewing this long ago. I had assumed that someone else
> was going to review. Thank you very much for working on this.

Thanks for taking the time to do a thorough review.

>  1311     // Params set to match the old behavior, though replace=false is
> probably a more sane default. 
>  1312     open("text/html", true);
> 
> I don't think the issue here is "sane default" but rather matching other
> browsers, since open is a public DOM function accessible from JavaScript. Can
> this change to replace = false? If not, then why not? This comment makes it
> seem like something is broken. Is something broken here? If so, what? What test
> would demonstrate the breakage?

I revised the comment, which I think was misleading.  Document::open() is not
called from JavaScript (the one with params is).  open() is called in various
places internal to WebCore, and I just wanted to preserve the old calling
defaults to avoid unintentionally changing anything.  An example callsite is
DOMImplementation::createHTMLDocument().

>     KURL generatedURL("generated:" + m_frame->document()->url());
> 
> Perhaps the URL scheme should also be changed to something with a vendor
> prefix. Just plain "generated:" seems like it might conflict with other uses.
> Something with webkit in it would probably be better. Can the URL used here be
> detected by JavaScript? If so, what effect will the URL have on site
> compatibility? What URL do you see on other web browsers in this case? Do any
> of the tests detect this generated URL?

I went with "webkitgenerated".  As far as I can tell, this url (used for the
substitute data's responseURL) is entirely internal to WebCore, and not visible
to the user or JavaScript at all.  The main reason in making it different is so
it looks like a different page to the FrameLoader, and we don't try to do a
reload.  Firefox uses "wyciwyg:" (what you cache is what you get).

I believe I took care of all your other suggestions.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.