Closed
Bug 1199934
Opened 9 years ago
Closed 9 years ago
Firefox allows site to prevent address bar editing (by repeatedly setting location.hash)
Categories
(Firefox :: Address Bar, defect)
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.)
Comment 1•9 years ago
|
||
Interesting, thanks! I can reproduce on current Nightly with the attached testcase.
Blocks: useragent
Status: UNCONFIRMED → NEW
Component: Untriaged → Location Bar
Ever confirmed: true
Keywords: reproducible,
testcase
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
Comment 2•9 years ago
|
||
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
Comment 4•9 years ago
|
||
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);
}
Comment 5•9 years ago
|
||
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 | ||
Updated•9 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(felipc)
Assignee | ||
Comment 6•9 years ago
|
||
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.
Assignee | ||
Comment 7•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/46165/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/46165/
Attachment #8741065 -
Flags: review?(jaws)
Updated•9 years ago
|
Attachment #8741065 -
Flags: review?(jaws)
Comment 8•9 years ago
|
||
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.
Assignee | ||
Comment 9•9 years ago
|
||
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 10•9 years ago
|
||
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+
Comment 11•9 years ago
|
||
Assignee | ||
Comment 12•9 years ago
|
||
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.
Comment 13•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Assignee | ||
Comment 14•9 years ago
|
||
(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.
Comment 15•9 years ago
|
||
verifying fixed in Mozilla/5.0 (Windows NT 10.0; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
Updated•9 years ago
|
Whiteboard: [bugday-20160727]
You need to log in
before you can comment on or make changes to this bug.
Description
•