Open Bug 803590 Opened 12 years ago Updated 2 years ago

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


(Core :: DOM: Navigation, enhancement)






(Reporter: torisugari, Unassigned)


(Blocks 3 open bugs)



(1 file)

The goal of this bug is to move

>  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.
Attached patch Proposal v1Splinter Review
>@@ -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.
Attachment #673323 - Flags: review?(bugs)
Do we really want to fix bug 220118? That would just complicate code and for what?
No opinion....
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.
Attachment #673323 - Flags: review?(bugs)
(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.

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.

Blocks: 709892
Blocks: 951685
See Also: → 801748
Blocks: 1150311
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.