Closed Bug 1199934 Opened 9 years ago Closed 8 years ago

Firefox allows site to prevent address bar editing (by repeatedly setting location.hash)

Categories

(Firefox :: Address Bar, defect)

40 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- verified

People

(Reporter: from_bugzilla3, Assigned: Gijs)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: regression, reproducible, testcase, Whiteboard: [bugday-20160727])

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20150819080340

Steps to reproduce:

1. Load http://gmtools.uo1.net/map.php
2. Focus the address bar and attempt to type a new URL


Actual results:

The site resets the address bar's contents under a second after a keypress is received.

(The family member who first encountered this initially blamed Firefox for having a buggy address bar and didn't even consider that it might be site-specific.)


Expected results:

Once the address bar receives a keypress other than Enter or cursor-moving keys (eg. backspace, printable characters), non-user updates to the address bar should be deferred.

(I remember "deferred until Escape is pressed" being the default behaviour somewhere else. I can't remember whether it's in Chrome or elsewhere in Firefox though.)
Interesting, thanks! I can reproduce on current Nightly with the attached testcase.
Blocks: useragent
Status: UNCONFIRMED → NEW
Component: Untriaged → Location Bar
Ever confirmed: true
OS: Unspecified → All
Hardware: Unspecified → All
Summary: Firefox allows site to prevent address bar editing → Firefox allows site to prevent address bar editing (by repeatedly setting location.hash)
Version: 42 Branch → Trunk
Attached file testcase
Have you tested previous versions to see if this is a regression?
This was caused by bug 1129564.
Regression range (I believe this can be called "regression"):
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=c6e4ebeb0e69d61998ab50e1d14fc4cd885cca17&tochange=806de9b02d86be6b7712a4959a76a88801e6f610
Blocks: 1129564
Has Regression Range: --- → yes
Has STR: --- → yes
Keywords: regression
Blocks: 1247816
See Also: → 798249
Blocks: eviltraps
Version: Trunk → 40 Branch
Might be able to do something like this in browser.js,
        // Don't change the URL bar contents if it is focused and
        // the location change is not a new navigation.
        if (!gURLBar.focused &&
            aWebProgress.isLoadingDocument &&
            !(aWebProgress.loadType & Ci.nsIDocShell.LOAD_CMD_PUSHSTATE)) {
          URLBarSetURI(aLocationURI);
        }
Oh, I didn't see the linked regression. Felipe, what do you think about comment #4 or tweaking the patch from bug 1129564?
Flags: needinfo?(felipc)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(felipc)
Pfff... I went back and forth on this quite a bit. You see, some of the documentation (https://dxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/browser.xml#776-814) in our code seems to imply that if you e.g.:

1. open https://en.wikipedia.org/wiki/Sweden
2. type in the location bar, say "aaaa"
3. scroll down to the table of contents and click one of the in-page links

the location bar should change.

But that didn't happen before bug 1129564 landed (tested on fx38), so I'm not trying to establish that behaviour here. We can try to add heuristics to do the "right" thing there, but that can be a separate bug if people want it.
Comment on attachment 8741065 [details]
MozReview Request: Bug 1199934 - page shouldn't be able to trap/revert the location bar by hash/replacestate changes, r?jaws

https://reviewboard.mozilla.org/r/46165/#review42677

This is a regression, so it would be nice if you included a test for it.
Comment on attachment 8741065 [details]
MozReview Request: Bug 1199934 - page shouldn't be able to trap/revert the location bar by hash/replacestate changes, r?jaws

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46165/diff/1-2/
Attachment #8741065 - Flags: review?(jaws)
Comment on attachment 8741065 [details]
MozReview Request: Bug 1199934 - page shouldn't be able to trap/revert the location bar by hash/replacestate changes, r?jaws

https://reviewboard.mozilla.org/r/46165/#review43755
Attachment #8741065 - Flags: review?(jaws) → review+
I triggered autoland on this, which is slightly interesting because 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d10b211d00c has quite some orange.

However, it turns out the parent changeset had the same orange, inasmuch as those jobs ran, when it landed on fx-team:

https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=c344a216f1d8&selectedJob=8634101

due to earlier commits.

So I'm assuming that that's it. I don't know why pushing to try (which I only did today) still pushes on a parent that's something like 5 days old. That's surprising, to say the least.
https://hg.mozilla.org/mozilla-central/rev/67cd0f4372e9
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
(In reply to :Gijs Kruitbosch from comment #6)
> Pfff... I went back and forth on this quite a bit. You see, some of the
> documentation
> (https://dxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/
> browser.xml#776-814) in our code seems to imply that if you e.g.:
> 
> 1. open https://en.wikipedia.org/wiki/Sweden
> 2. type in the location bar, say "aaaa"
> 3. scroll down to the table of contents and click one of the in-page links
> 
> the location bar should change.
> 
> But that didn't happen before bug 1129564 landed (tested on fx38), so I'm
> not trying to establish that behaviour here. We can try to add heuristics to
> do the "right" thing there, but that can be a separate bug if people want it.

This is bug 435932, it turns out. People seem to disagree there and in the tail end of bug 302575 about what should happen in that case.
verifying fixed in Mozilla/5.0 (Windows NT 10.0; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
Whiteboard: [bugday-20160727]
Depends on: 435932
Blocks: 1602991
You need to log in before you can comment on or make changes to this bug.