Closed
Bug 41973
Opened 24 years ago
Closed 24 years ago
Redirected links are not marked Visited
Categories
(Core :: DOM: Navigation, defect, P4)
Core
DOM: Navigation
Tracking
()
VERIFIED
FIXED
mozilla0.8
People
(Reporter: pierre, Assigned: radha)
References
()
Details
(Keywords: embed, top100, Whiteboard: [nsbeta2-][nsbeta3-][PDTP4])
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
- Go to Yahoo
- Click almost any link
- Go back
==> The link is not marked as visited
I guess it is related to the fact that these links are redirected. For instance,
if you click "Business & Economy", it goes to http://www.yahoo.com/r/bu which is
redirected to http://dir.yahoo.com/Business_and_Economy/
Assignee | ||
Comment 2•24 years ago
|
||
waterson, Do you have any clue on this one? Thanks,
Comment 3•24 years ago
|
||
it's probably not being added to the global history. check the flow of control
in the docshell stuff.
Putting on [nsbeta2-] radar. Not critical to beta2. Adding nsbeta3 keyword.
Keywords: nsbeta3
Whiteboard: [nsbeta2-]
Reporter | ||
Comment 5•24 years ago
|
||
Moving some of my comments from bug 12493...
----
Bug 41973 comes from the assumption that a page that causes a redirection isn't
added to the History before causing the redirection. But as I'm writing this, I'm
realizing that Marc and I could have been wrong because the URLs on Yahoo are not
canonized either. We need to write a testcase with canonized URLs that cause a
redirection. If it works, then bug 41973 is in fact a dup of bug 12493.
nav triage team:
if this also effects Back / Fwd, we will definitely nsbeta3+ it.
By the way, this effects almost all portals like Yahoo, Netscape.com, etc.
Whiteboard: [nsbeta2-] → [nsbeta2-][NEED INFO]
nav triage team:
Claudius says it does not effect Back / Forward button - doesn't appear to.
However, this does mean that links on about every portal will nto change colors
to show that you have visited the page. That is bad, marking nsbeta3+
(borderline, so if it is hard to fix, we might revisit this).
Whiteboard: [nsbeta2-][NEED INFO] → [nsbeta2-][nsbeta3+]
Assignee | ||
Comment 8•24 years ago
|
||
The fix for it needs change in docshell. Not in Session History. Changing
component.
Assignee: radha → valeski
Component: History → Embedding: Docshell
Comment 9•24 years ago
|
||
Radha, do we change the color of links based on history entries? If so, I don't
think the docshell will ever know about a redirect as they happen transparently
(right gagan?) in necko. This would imply that http would need to set the
history :-/.
Assignee | ||
Comment 10•24 years ago
|
||
visited link color change is done by global history owned by rjc. But I think
the code that lets global history know about such links exists in docshell.
There is a inherent problem with redirected links though. Redirected links are
passed to docshell with loadtype "loadNormal" which gives rise to a bunch of
sesion History related bugs. Gagan has a bug on this. May be this problem is
related to the bug that gagan has.
Updated•24 years ago
|
Target Milestone: --- → M18
Comment 11•24 years ago
|
||
I don't see how the loadNormal bug is related to this bug. The essential problem
as I see it is that history is not updated for visited links (thru redirects)
and the main reason is becuz webshell/docsheel needs to update that just as they
would update the location bar on a redirect. cc'ing mscott.
Comment 12•24 years ago
|
||
Okay so on a whim I implemented nsIHTTPEventSink tonight 'cause we seem to have
a set of bugs that we think would be fixed by such an implementation.
Looking at this bug after implementing http event sink, I'm not sure how that's
supposed to fix this. The docshell gets told of the location change. It looks
like the new url is added correctly to history.
The problem is really that we don't add urls to history until after we load
them. i.e. until after the redirection happens....so we only end up adding the
final url.
Comment 13•24 years ago
|
||
so who's impl'ing nsIHTTPEnventSink? I suspect whoever's doing that should add
the original url (the one that caused the redirect) to history when the
HTTPEventSink gets redirect notification (maybe this is in the form of checking
the HTTP status code?). Scott, from your comments I can't tell if this is
already happening or not?
Assignee | ||
Comment 14•24 years ago
|
||
*** Bug 44560 has been marked as a duplicate of this bug. ***
Updated•24 years ago
|
Priority: P3 → P1
Comment 15•24 years ago
|
||
If this is truly only a lack of visual indication, it can wait for the next
release cycle.
Priority: P1 → P4
Whiteboard: [nsbeta2-][nsbeta3+] → [nsbeta2-][nsbeta3+][PDTP4]
Comment 16•24 years ago
|
||
Not holding PR3 for this; marking nsbeta3-.
Whiteboard: [nsbeta2-][nsbeta3+][PDTP4] → [nsbeta2-][nsbeta3-][PDTP4]
Comment 17•24 years ago
|
||
*** Bug 59304 has been marked as a duplicate of this bug. ***
Comment 18•24 years ago
|
||
*** Bug 58642 has been marked as a duplicate of this bug. ***
Comment 19•24 years ago
|
||
*** Bug 60211 has been marked as a duplicate of this bug. ***
Updated•24 years ago
|
Target Milestone: M18 → mozilla0.8
Assignee | ||
Comment 20•24 years ago
|
||
Looking thro' the comments, I see that the problem described here occurs for all
server side redirects. Necko handles the redirection and docshell has absolutely
no info on it. So the original url never gets added to global history, which
means no color change for the original url. nsDocLoader implements
nsIHTTPEventSink and it sends out a nsIWebProgressListener::onStateChange()
notification when there is a redirect on the main document channel. DocShell
implements nsIWebProgressListener. Little debugging should provide info on
whether this notification comes on time to update the history.
Re-assigning to self.
Assignee: valeski → radha
Comment 21•24 years ago
|
||
*** Bug 61223 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 22•24 years ago
|
||
Comment 23•24 years ago
|
||
(r=rpotts)
hey Radha,
This patch looks fine... You're correct that when the document URI gets
redirected, OnStateChange(...) is called with STATE_REDIRECTING |
STATE_IS_DOCUMENT flags set...
-- rick
Comment 24•24 years ago
|
||
sr=waterson; looks good to me.
Comment 25•24 years ago
|
||
Rick's right, you are using the correct status flags. sr=mscott
Comment 26•24 years ago
|
||
Not raising any flag or demanding any change, but trying to be consistent: I
recently persuaded/nagged adam lock into optimising (flags & F1) && (flags & F2)
into (~flags & (F1|F2)) == 0. Same applies here:
+ else if ((aStateFlags & STATE_IS_DOCUMENT) && (aStateFlags &
STATE_REDIRECTING)) {
could be, maybe should be:
+ else if ((~aStateFlags & (STATE_IS_DOCUMENT | STATE_REDIRECTING)) == 0) {
saves a branch and second mask test, compares to zero for faster test on many
architectures. jst used this pattern recently, too.
/be
Assignee | ||
Comment 27•24 years ago
|
||
This was fixed last week
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•