359857 - Attempting to go back/forward to a POSTDATA page in session history fails (and POSTDATA warning missing)

Status: VERIFIED FIXED

(Camino Graveyard :: History, defect, P4)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

STR:

1. Go to a page that has an HTML form login
2. Login
3. Follow a link off of your login landing page
4. Observe the back button is active, indicating session history
5. Press the back button

Expected: POSTDATA error sheet (and option to go back or cancel)
Actual: Fails silently (nothing happens when pressing back button)

The same thing happens if you choose a page further back in session history and attempt to go forward from there.

This worked fine in my trunk build from yesterday, so I suspect bug 112848 (which is also using embedding-hostile strings).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Any useful console output in a debug build?  Is this just due to there being no brand.properties (which would cause us to bail out)?

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Unhelpful console spew

Nothing at all gets written in the console when I press the back button :(

In case it's useful at all, here's the log from pressing the Login button until pressing the Back button (marked sections for those 2 and for the "click link" action). 

Typically, though, we just get "(null)" in the error sheets' messages (where Fx has "Firefox") instead of failing completely....  I can't remember if we got console spew in that case (and webmail.shaw.ca seems to have upgraded their ciphers, so I'm not sure of a good testcase for the weak ciphers/no matching ciphers version of the error anymore).

Comment 3

[Boris Zbarsky [:bzbarsky]]

Would you be willing to step through the docshell code in question and see what fails?

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I'll find someone who can use a debugger (or at least walk me through it); Stuart? ;)

Comment 5

[Stuart Morgan]

Sorry, yet another thing that fell off my radar when I got busy.

We are getting a return value of 0x8000FFFF (NS_ERROR_UNEXPECTED) from:
  rv = brandBundle->GetStringFromName(NS_LITERAL_STRING("brandShortName").get(),
                                      getter_Copies(brandName));
in ConfirmRepost.

I find it worrying that at our interface layer, our call to nsIWebNavigation::GoBack() is returning 0, giving us no indication that going back in fact totally failed. I haven't traced the return value yet, but philosophically that seems pretty wrong.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Navigation is async.  GoBack() returns, then some time later (necko is always async) HTTP tells docshell that the data was not cached.  At that point docshell tries to put up that post data dialog.  So there's no way for GoBack() to throw in this case.

Should this code just press on without a brandShortName perhaps?  How hard would it be for Camino to end up with one, since this is a recurring issue?

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #6)
> Should this code just press on without a brandShortName perhaps?  How hard
> would it be for Camino to end up with one, since this is a recurring issue?

I understood the discussion in bug 302309 to have reached a consensus that requiring embedding apps to ship branding was not really desirable and that shipping a default branding package for embeddors was not feasible currently.  

IMO, having Camino ship branding would only push detection of these bugs out of mozilla.org and on to external embeddors, which doesn't seem like a good resolution, but if it's easy enough for us to do so (so far no one with chrome/jar-fu has had a look, to my knowledge--we have bug 354013 on that, but most discussion was actually in bug 343837 comment 10-15), it _would_ make bugs like this magically vanish for us.

Comment 9

[Mike Schroepfer]

marking as blocking (since it is a regression) with p4 (since it seems minor)

Comment 10

[Stuart Morgan]

(In reply to comment #9)
> with p4 (since it seems minor)

How exactly is it minor that any page with post data will completely break history navigation for any product that doesn't ship branding (which is explicitly not a requirement; see comment 7)?

Comment 11

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

With the checkin for bug 392407, this is now working, so FIXED.  

Is there anything in comment 5/comment 6 that still needs to be addressed (if so, probably in a new bug)?

Comment 12

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Er, actually clicking the radio button :p

Comment 13

[Stuart Morgan]

Nope, comment 5 was just confusion on my part about the expected behavior of the API.