As discussed at http://etherpad.mozilla.com:9000/kwpLzwwtbz , we'd like URL after processing a bug not to be process_bug.cgi but to be the regular bug URL. We could use history.replaceState() to accomplish this pretty easily.

Note that this function doesn't exist until Firefox 4 according to the docs. However, it's possible the YUI History Manager could do it now.

The fact that bug submission leads to process_bug.cgi rather than show_bug.cgi is a long-standing issue (bug 427913, bug 365078). It's not simple to solve. Basically, this bug's suggestion to fix it involves adding something like the following code to the output of process_bug.cgi: <script> if (history.replaceState) { history.replaceState(null, null, "show_bug.cgi?id=[% bug.id %]") } </script> This fixes the problem in Firefox, at very low cost and with no need to rearchitecture. Gerv

(In reply to comment #1) > Note that this function doesn't exist until Firefox 4 according to the docs. FWIW, the latest versions of Chrome and Safari have both shipped push/replaceState.

Okay. So this does sound like a great and simple way to handle this problem. Is this part of the HTML5 spec still in flux? Aren't there security concerns about modifying the URL in the title bar without modifying the actual page being displayed?

I suspect if two browsers have implemented it, it's stable enough for us to use. The security concerns are addressed by the fact that this call raises an exception if the same-origin policy is violated. That is to say, you can only change the URL to other URLs in the same domain. (You could do something similar by redirecting to "#foo" in older browsers. This just extends a bit the parts of the URL you can hack.) Gerv

Make sure when this is implemented that it also handled attachments as well as bug saves. Obviously this won't work for people who have JS turned off. Do we know how this works with bookmarks etc?

(In reply to comment #5) > I suspect if two browsers have implemented it, it's stable enough for us to > use. Well, I wouldn't be sure about that until it's in a CR, based on the degree to which other aspects of HTML5 have fluxed. > The security concerns are addressed by the fact that this call raises an > exception if the same-origin policy is violated. That is to say, you can only > change the URL to other URLs in the same domain. Okay. :-) I just wanted to be sure that there wasn't going to be some future security restriction that prevented us from doing what we want to do.

BTW, If somebody does this relatively soon, and it's really just those three lines of code that could go into show-header, then I'd be happy to take it for 4.0.

Comment on attachment 456633 [details] [diff] [review] v1 That's wrong in several ways :-) First, it should be matching process_bug.cgi, not show_bug.cgi. Then, how do you know that $javascript is semicolon-terminated? I ask particularly because it's no longer semicolon-terminated after you've finished with it! So how are you relying on everyone else to do the right thing? Lastly, we need to do it for a number of different URLs, as Max indicates. I'm not quite sure why Max said it should be in the header, as it's specific to particular CGIs. Wouldn't it make more sense to have it in the main template served for that CGI? OK, we'd need a few copies of it with different target URLs, but it's only one line of code. Did you check that '' is a valid value for the first parameter? It wasn't immediately clear to me from the docs... $title will be "Bug processed"; it would be much better if the title set were the same title that the bug normally has... Gerv

(In reply to comment #10) > First, it should be matching process_bug.cgi, not show_bug.cgi. No, actually he's correct there. This is in show-header, which sets up header.html.tmpl for any time that edit.html.tmpl is going to be included. > Then, how do you know that $javascript is semicolon-terminated? That's not necessary because it doesn't already exist at this point. > Lastly, we need to do it for a number of different URLs, as Max indicates. show-header handles all those, so it's OK. > Did you check that '' is a valid value for the first parameter? It wasn't > immediately clear to me from the docs... This is something we should confirm, and probably the MDC docs should be clarified if it's not clear. > $title will be "Bug processed"; it would be much better if the title set were > the same title that the bug normally has... I think that might be better as a more general change to the templates.

Comment on attachment 456633 [details] [diff] [review] v1 >=== modified file 'template/en/default/bug/show-header.html.tmpl' >+ [% javascript = "$javascript window.history.replaceState('','$title', 'show_bug.cgi?id=$bug.bug_id' )" %] Are you using $javascript because calling templates use it? Perhaps you should prefix your code to $javascript instead, if so. (I see what Gerv was talking about now, sorry Gerv.) You need to do FILTER js on $title before inserting it into the string, or there could be script injections. Also, this needs to only be done "if window.history.replaceState". Also, are you sure it's "window" that you want? Theoretically we could be in a frame, no?

fixed the concerns as well as made sure it wasn't too long in length.

in a block now

(In reply to comment #6) > Do we know how this works with bookmarks etc? If you can't bookmark the page after calling replaceState, or if the URL of the bookmark doesn't reflect the URL you set by calling replaceState, that's a bug. (In reply to comment #11) > Did you check that '' is a valid value for the first parameter? It wasn't > immediately clear to me from the docs... Any object which can be passed to JSON.stringify works in Firefox. The spec's requirement is even looser. So certainly '' works. Also null should work.

Bah, could you please dupe this bug when you know it's a dupe? See comment 2. There are 24 people missing this discussion because you keep discussing in this bug instead of the original ones.

(In reply to comment #17) > Bah, could you please dupe this bug when you know it's a dupe? See comment 2. > There are 24 people missing this discussion because you keep discussing in this > bug instead of the original ones. I commented in both of those bugs directing them here. I don't think this is strictly a dupe, since it only solves the problem on browsers which support this API.

(In reply to comment #1) > Note that this function doesn't exist until Firefox 4 according to the docs. > > However, it's possible the YUI History Manager could do it now. Will this work in any Gecko browsers prior to Firefox 4 (which, AIUI, is now coming from Gecko 1.9.3+; please correct me if I'm wrong there)? If not, we still need a proper solution for bug 427913.

(In reply to comment #19) > Will this work in any Gecko browsers prior to Firefox 4? No.

Comment on attachment 456651 [details] [diff] [review] V3 >=== modified file 'template/en/default/bug/show-header.html.tmpl' >+ if( history && history.replaceState ) { >+ var null_state = {}; >+ history.replaceState( null_state , I'm pretty sure you can just literally pass "null" for that first argument. >+ "show_bug.cgi?id=[% bug.bug_id FILTER js %]" ); I think you also need to wrap the whole block in an "IF bug.defined" because otherwise, when updating a bug where there is no next item in the list, I get a URL like: https://landfill.bugzilla.org/mkanat/show_bug.cgi?id=

I just realized that even with this change, if you submit a change to a bug and then refresh the page, you'll probably still get a warning that you're refreshing a POST'ed page. This might be a bit confusing, since it will appear from the URL that we didn't POST to the page. But it's probably not a big deal.

You do get a warning. It is confusing, I don't know anything about the whole "replaceState" capability so if you can tell me how to fix it I'd be happy to include it in the patch. The best part about this addition is that bookmarking works and refresh doesn't become totally ridiculous. At least it doesn't try to resubmit your changes.

(In reply to comment #23) > You do get a warning. It is confusing, I don't know anything about the whole > "replaceState" capability so if you can tell me how to fix it I'd be happy to > include it in the patch. push/replaceState doesn't help you get around the warning. My understanding is that you get the warning when you refresh a document that was POST'ed to. Using replaceState to change the URL doesn't change the fact that you POST'ed to process_bug.cgi. > At least it doesn't try to resubmit your changes. It doesn't? Isn't that what the warning is about? Or does Bugzilla have a way to make the "Save Changes" POST idempotent?

I'd say that that POST issue is a bug (or deficiency) in the History API or the browser, probably, unless there is some good and reasonable way to work around it.

(In reply to comment #25) > I'd say that that POST issue is a bug (or deficiency) in the History API or > the browser, probably, unless there is some good and reasonable way to work > around it. Yeah, the more I think about it, I think this might be a spec bug. I'll bring it up with WhatWG.

See also bug 580069, which I'm going to check in soon (though not for FF4b4). This should fix the refresh problems.

Note also that the title parameter to replaceState is completely ignored (per bug 544535), so if you want to change document.title, you'll have to set it explicitly. I imagine you do want to change the title here so it's not "Bug N was processed".

Comment on attachment 465855 [details] [diff] [review] v4 >+ history.replaceState( "null", Why are you passing the string "null" instead of an actual null? >+ "[% title FILTER js %]", It sounds like the API of pushState may be changing too rapidly for us to use it at the moment, if the title argument is going away or being ignored. I suppose we have to decide what to do here about the title, and I'd like to see some decision from the WhatWG about it.

I'd either pass the empty string as the title or pass what you want document.title to be as the title. Since Chrome and Safari have both shipped replaceState and sites are already using it, the API isn't going to change. I think it's absolutely stable enough to use. There's been some talk about using the title parameter to change document.title, but I don't think it's going to happen at this point. Even if it does, passing the either empty string or the title you actually want will be OK.

@jlebar Okay, I read over the bit about the title parameter, and I think it's unfortunate, because instead of fixing that problem, browser implementors are effectively pushing it off onto every single consumer of replaceState, no? What are we supposed to do with the problem of title and back/forward?

I'd love to talk about the title parameter, but I don't think this bug is the place to do it. How about bug 585653?

feel free to not approve etc but I think this has what you want.

Comment on attachment 465896 [details] [diff] [review] 4.1 Yeah, the upside is way better than the downside, here. :-)

Don't you need to set document.title?

Well, not according to the spec.... ;-)

(In reply to comment #37) >> Don't you need to set document.title? > Well, not according to the spec.... ;-) You can still set the title the old-fashioned way (document.title = "...";).

(In reply to comment #37) > Well, not according to the spec.... ;-) No, actually. See [1]: "The title is purely advisory. User agents might use the title in the user interface." Ignoring it isn't optimal, but it's definitely compliant with the spec. And certainly the spec doesn't suggest that the title argument is linked to document.title. [1] http://www.whatwg.org/specs/web-apps/current-work/multipage/history.html#dom-history-pushstate

Ah, okay. pyrzak, do you want to submit another patch that updates document.title also?

sure. why not.

can't commit this until bug 588483 is fixed.

Comment on attachment 466358 [details] [diff] [review] 4.1 with document.title = new Title >+ [% javascript = BLOCK %] >+ if( history && history.replaceState ) { >+ history.replaceState( null, >+ "[% title FILTER js %]", >+ "show_bug.cgi?id=[% bug.bug_id FILTER js %]" ); >+ document.title = "[% title FILTER js %]"; The filtering of the title is wrong. HTML entities are all escaped, so you e.g. see &ndash; &quot; instead of - and ". >+ [% javascript FILTER none %] >+ [% END %] Is it intentional that [% javascript %] is *inside* [% javascript = BLOCK %]? This looks wrong to me. Also, please fix the indentation.

Comment on attachment 480354 [details] [diff] [review] filtered fixed I'm pretty sure the second one shouldn't have an &ndash; in it, right?

Comment on attachment 480458 [details] [diff] [review] fixed escaped endash Nice! The POST resubmission problem still exists, of course, but that's a browser-side issue.

Committing to: bzr+ssh://guy.pyrzak%40gmail.com@bzr.mozilla.org/bugzilla/trunk/ modified template/en/default/bug/show-header.html.tmpl Committed revision 7514.

(In reply to comment #48) > The POST resubmission problem still exists, of course, but that's a > browser-side issue. We fixed that in bug 580069. Hopefully WebKit will follow suit.

Note that this bug is responsible for bug 652748.

Added to relnotes in bug 713346.