Closed Bug 1624550 Opened 1 year ago Closed 10 months ago

Redirect-to-download links no longer marked as visited


(Core :: Networking, defect, P1)




Tracking Status
firefox-esr68 --- unaffected
firefox75 --- wontfix
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- verified


(Reporter: emk, Assigned: u480271)




(Keywords: regression, Whiteboard: [necko-triaged])


(5 files)

Steps to reproduce:

  1. Navigate to
  2. Click the first link to download a file.

Actual result:
The link color is not changed.

Expected result:
The link color should be changed.

Regression range:

Flags: needinfo?(matt.woodrow)
Priority: -- → P1
Whiteboard: [necko-triaged]

This happens because the redirect is handled by DocumentLoadListener, and then we handle the final URI entirely in the parent using ParentProcessDocumentOpenInfo.

We notify the originating docshell that the load has completed via PDocumentChannel::DisconnectChildListeners, which reports the status code, but the redirect isn't exposed. The docshell thinks that the initial load failed, so records no history entries. The download handler adds the entry for the final downloaded URI.

It looks like IHistory::VisitURI just sends the values it gets back to the parent, and doesn't do any handling in the content process.

Given that, I think we should make DocumentLoadListener responsible for calling VisitURI (on redirects, and when it decides to deliver to the docshell), stop sending the redirect list to the content process, and disabling the VisitURI calls in docshell when we're using DocumentChannel.

Flags: needinfo?(matt.woodrow)
Assignee: nobody → dglastonbury

I'm working on updating history in the parent process from DocumentLoadListener.

This code was relying on SessionHistory update having completed when
GlobalHistory is update. Moving GlobalHistory update to the parent process with
DocumentChannel means that it's possible for the GlobalHistory event to fire
before the SessionHistory is updated in nsDocShell.

Switch to using tab loading complete to signal when it's OK to check

For use by both nsDocShell and DocumentLoadListener.

Avoid round-trip Parent->Content->Parent to add visited URI to browsing history.

This value is determined in Parent process and passed down to nsDocShell. Delete
the messages to pass the setting down and set it on the BrowsingContext in the
Parent process.

Refactor the code that determines to opt-out of using global history. Code
inspection determines that windowless browsing contexts want to opt-out as well
as any frame with disableglobalhistory attribute set on it.

Blocks: 1425410
Pushed by
P1: Move nsDocShell::mUseGlobalHistory to BrowsingContext. r=farre
P2: Extract AddURIVisit internals into a helper. r=mak,farre
P3: Update Global History from DocumentLoadListener in Parent process. r=mak,farre,mattwoodrow
P4: Cleanup APIs for setting BrowsingContext::UseGlobalHistory. r=farre
P5: Fix racy check for SessionHistory update. r=kmag

The patch landed in nightly and beta is affected.
:djg, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(dglastonbury)
Flags: needinfo?(dglastonbury)
Flags: qe-verify+

Reproduced the issue using Firefox 76.0a1 (20200324214641) on Windows 10x64. The issue is verified fixed with Firefox 78.0b7 on Windows 10x64, macOS 10.12 and Ubuntu 18.04.

Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.