803590 - Rename onRefreshAttempted() to onRedirectAttempted() and make it handle all the redirection

Status: UNCONFIRMED

(Core :: DOM: Navigation, enhancement)

Description

[O. Atsushi (Torisugari)]

The goal of this bug is to move

>bool
>nsDocShell::ShouldBlockLoadingForBackButton()
>{
>  if (!(mLoadType & LOAD_CMD_HISTORY) ||
>      nsEventStateManager::IsHandlingUserInput() ||
>      !Preferences::GetBool("accessibility.blockjsredirection")) {
>    return false;
>  }
>
>  bool canGoForward = false;
>  GetCanGoForward(&canGoForward);
>  return canGoForward;
>}

from nsDocShell.cpp to browser.js. That would make it easier to fix bug 220118 etc. and to show infobar.

Comment 1

[O. Atsushi (Torisugari)]

Attachment: Proposal v1

>@@ -1402,19 +1403,23 @@ nsDocShell::LoadURI(nsIURI * aURI,
>-    if ((loadType == LOAD_NORMAL || loadType == LOAD_STOP_CONTENT) &&
>-        ShouldBlockLoadingForBackButton()) {
>-        return NS_OK;
>+    if ((loadType == LOAD_NORMAL || loadType == LOAD_STOP_CONTENT)) {
>+        uint32_t redirectFlags = (loadType == LOAD_NORMAL)?
>+            nsIWebProgressListener2::REDIRECT_DOM_OPEN:
>+            nsIWebProgressListener2::REDIRECT_DOM_LOCATION;
>+        if (!IsRedirectAllowed(aURI, redirectFlags)) {
>+            return NS_OK;
>+        }

Previously I intentionally ignored LOAD_STOP_CONTENT_AND_REPLACE because such a redirect never breaks back button. But maybe bug 220118 requires blocking location.replace() too...

+        if (LOAD_REFRESH != mLoadType) {
+            redirectFlags |= nsIWebProgressListener2::REDIRECT_IS_FIRST_REFRESH;
+        }
For bug 417770.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Do we really want to fix bug 220118? That would just complicate code and for what?

Comment 3

[Boris Zbarsky [:bzbarsky]]

No opinion....

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 673323 [details] [diff] [review]
Proposal v1

Clearing the review request until I understand why we want to make all these changes and add yet some more flags.

Comment 5

[O. Atsushi (Torisugari)]

(In reply to Olli Pettay [:smaug] from comment #4)
> Clearing the review request until I understand why we want to make all these
> changes and add yet some more flags.

Personally I think it's nice to fix bug 220118. It would become more and more important to improve a11y if Mozilla keeps supporting poor UI devices, like tablet machines and smart phones. And you can read a awesomely long post about this topic at bug 788497, if you have enough time.

Bug 687300 is, in other words, we need a reason why |location.href| does not popup the infobar. WCAG-1.0 says web developers should not use content-side redirection like location.href "until user agents provide the ability to stop auto-redirect". But we can't ask web developers not to use content-side redirection. Then all what we can do is to "provide the ability to stop auto-redirect", in behalf of encouraging WCAG.
http://www.w3.org/TR/WCAG10-TECHS/#tech-no-auto-forward

I think we should pay enough attention to make docshell (or any other code) lean and mean, but I believe this is a valid bug.

?