728939 - document.URL is not updated when document's fragment changes

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Karl]

Attachment: docurlfragbug.xpi

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/17.0.963.46 Safari/535.11

Steps to reproduce:

The attached XPT add-on provides a context menu item that displays the URL of a page, as retrieved by "document.URL". A specific repro procedure follows:
1. Run Firefox with the attached XPT installed.
3. Into the Firefox GUI enter the URL 
	https://developer.mozilla.org/en/New_in_JavaScript_1.7
and display the page.
4. Right-click on an empty area of the page and select the menu item "Display URL". 
5. Compare the displayed URL with the URL previously entered into the GUI.
6. In the "TABLE OF CONTENTS" column on the right of the page, left-click on "Generators".
7. Right-click on an empty area of the page and select the menu item "Display URL". 
8. Compare the displayed URL with the URL now appearing in the GUI.
9. Using the left-arrow button, navigate back to the previous page (the page before "Generators" was clicked.
10. Using the right-arrow button, navigate forward (which is now back to the second "Generators" page).
11. Right-click on an empty area of the page and select the menu item "Display URL".
12. Compare the displayed URL with the URL now appearing in the GUI.
13. In the "TABLE OF CONTENTS" column on the right of the page, left-click on "Iterators".
14. Right-click on an empty area of the page and select the menu item "Display URL".
15. Compare the displayed URL with the URL now appearing in the GUI.
16. Using the left-arrow button, navigate back to the previous page.
17. Using the right-arrow button, navigate forward (which is now back to the "Iterators" page).
18. Compare the displayed URL with the URL now appearing in the GUI.

The add-on SDK script is simple, it reads:

var contextMenu = require("context-menu");

exports.main = function(options, callbacks) {
  console.log(options.loadReason);
 
  var menuItem = contextMenu.Item({
    label: "Display URL",

    contentScript: 'self.on("click", function () {' +
		   '  alert( \"value of  \'document.URL\'  is: \" + document.URL ); ' +
                   '});'
  });
};



Actual results:

Step 5 results:  Strings are identical.
Step 8 results:  Strings are not identical. The "document.URL" still has the URL of the previous page, i.e. the fragment identifier is missing from the programmatically accessed URL.
Step 12 results:  Strings are identical. The navigation away from the page and then back resulted in agreement between the displayed URL and the programmatically accessed URL.
Step 15 results:  Strings are not identical. The "document.URL" still has the URL of the previous page, i.e. the fragment identifier is "#Generators" and not "#Iterators".
Step 18 results:  Strings are identical. The navigation away from the page and then back resulted in agreement between the displayed URL and the programmatically accessed URL.



Expected results:

The string comparisons as described should always agree. Moreover, the "document.URL" value should be unaffected by user actions to display some other page.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This sounds like we should just SetRef on the document's documentURI when scrolling to ref, right?

Comment 2

[Alice0775 White]

Cannot reproduce, because # fragment is always missing:
http://hg.mozilla.org/mozilla-central/rev/6e3003aeea75
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100201 Minefield/3.7a1pre ID:20100201085418
Can reproduce:
http://hg.mozilla.org/mozilla-central/rev/5c6c40887584
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100201 Minefield/3.7a1pre ID:20100201174142
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6e3003aeea75&tochange=5c6c40887584

In local build:
afff5c13d296 : Cannot reproduce, because # fragment is always missing
a2f468118868 : Can reproduce

Suspected:
a2f468118868	Justin Lebar — Bug 500328 - Implement History.pushState(), History.replaceState() methods. r=smaug, r=zpao, sr=sicking

Comment 3

[Karl]

(In reply to Boris Zbarsky (:bz) from comment #1)
> This sounds like we should just SetRef on the document's documentURI when
> scrolling to ref, right?

Perhaps that would work if it were done whether or not an actual scroll is undertaken, but I am not sure; would that fix such problems for example with Google's hashbangs and other places where the URI following the # is not a standard fragment identifier, may not stimulate a scroll, and is more significant to page content or content visibility than the URI specification originally intended? 
Background for my question: 
Not all fragment identifiers are intended to be processed by the browser and do not actually cause a scroll because they do not identify a location on the page. In such cases, even though they are not sent to the page server with the rest of the URI by the browser per-se, they are nevertheless processed by scripts and can entirely change the page content downloaded. This is prevalent in AJAX accesses where the real identifier of page content often follows the #. It should be noted that the Google hashbang "specification" explicitly allows for additional '#' characters to follow the '#!' (hashbang). If the page server does not need or desire Google indexing of their site or a page, they can do all of this kind of thing without a hashbang, i.e. with only the use of '#'. The widespread use of this capability, albeit outside the intent of the original URI specifications, is what caused Google to create the hashbang, so that they could more accurately index the content behind such URIs. The stuff that comes after the # is increasingly important and semantically diverse.

Comment 4

[Justin Lebar (not reading bugmail)]

> Perhaps that would work if it were done whether or not an actual scroll is undertaken, 
> but I am not sure

I think bz meant that we'd set the ref even if there wasn't a hash to scroll to.

Comment 5

[Justin Lebar (not reading bugmail)]

Attachment: Minimal testcase

Minimal testcase.  Load this page, click #foo.  document.location.href updates, but document.URL does not.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> I think bz meant that we'd set the ref even if there wasn't a hash to scroll to.

Yes, precisely.

Comment 7

[Karl]

(In reply to Boris Zbarsky (:bz) from comment #6)
> > I think bz meant that we'd set the ref even if there wasn't a hash to scroll to.
> 
> Yes, precisely.

OK, thanks. Also perhaps worth mentioning is that this could possibly be affected by, or be a symptom of, a more general issue of navigation history improperly affecting page state. A possible example is Bug 721849, which has in common with this bug that the navigation means to arrive at a page improperly affects what appears to be the final page state.

Comment 8

[Justin Lebar (not reading bugmail)]

Attachment: Part 2, v1: Tests

Comment 9

[Justin Lebar (not reading bugmail)]

Attachment: Part 1, v1

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 599594 [details] [diff] [review]
Part 2, v1: Tests

location.href is clearer than location + ''.  r=me with that.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 599595 [details] [diff] [review]
Part 1, v1

r=me

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/fda0b21150b3
https://hg.mozilla.org/mozilla-central/rev/8e8c0e692fa4