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

UNCONFIRMED
Unassigned

Status

()

--
enhancement
UNCONFIRMED
6 years ago
4 years ago

People

(Reporter: torisugari, Unassigned)

Tracking

(Blocks: 3 bugs)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
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.
(Reporter)

Comment 1

6 years ago
Created attachment 673323 [details] [diff] [review]
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.
(Reporter)

Updated

6 years ago
Attachment #673323 - Flags: review?(bugs)

Comment 2

6 years ago
Do we really want to fix bug 220118? That would just complicate code and for what?
No opinion....

Comment 4

6 years ago
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)
(Reporter)

Comment 5

6 years ago
(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.

?
Blocks: 709892
(Reporter)

Updated

5 years ago
Blocks: 951685

Updated

4 years ago
See Also: → bug 801748

Updated

4 years ago
Blocks: 1150311
You need to log in before you can comment on or make changes to this bug.