956576 - Location app bar pops up when fragment identifier changes (URL stuff after hash / number sign)

Status: VERIFIED FIXED

(Firefox for Metro Graveyard :: General, defect)

Description

[flamingdescent]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20140105004002

Steps to reproduce:

1. Go to http://www.haikudeck.com/p/IXPQ9thNi1/10-tips-to-transform-your-presentations#
2. To dismiss the bottom app bar, left click somewhere that isn't a link.
3. Press the next button on the webpage
If you have a Google Account, you can also:
1. Go to http://www.google.com/maps/about/explore/
2. Press "Try it now" on the webpage
3. Press "Sign In" on the webpage, and sign in
4. Press "Click here to start exploring"
5. Drag somewhere to move the map.


Actual results:

The bottom app bar pops up, probably because the URL changed.


Expected results:

The bottom app bar doesn't pop up.

Comment 1

[Matt Brubeck (:mbrubeck)]

This happens because in the URLChanged event listener here:
http://hg.mozilla.org/mozilla-central/file/cf80c0d4f46e/browser/metro/base/content/ContextUI.js#l318

We should change that code to display the navbar only if aEvent.detail is true.  We set the "detail" property based on the locationHasChanged variable here:
http://hg.mozilla.org/mozilla-central/file/cf80c0d4f46e/browser/metro/base/content/WebProgress.js#l119

Comment 2

[Oleg Zasypkin [:azasypkin]]

Attachment: location appbar v1.diff

Comment 3

[Matt Brubeck (:mbrubeck)]

Comment on attachment 8361778 [details] [diff] [review]
location appbar v1.diff

Review of attachment 8361778 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Could you also add an automated test for this bug?  You can do this in a separate patch.  The test can go in this file:
http://dxr.mozilla.org/mozilla-central/source/browser/metro/base/tests/mochitest/browser_context_ui.js

These tests use the mochitest-browser-chrome framework, which is documented here:
https://developer.mozilla.org/en-US/docs/Browser_chrome_tests
https://wiki.mozilla.org/Firefox/Windows_8_Integration#Testing

Comment 4

[Oleg Zasypkin [:azasypkin]]

Attachment: location appbar tests.diff

Tests added.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/cec5c572a516
https://hg.mozilla.org/integration/fx-team/rev/6dfa253da690

In the future, please use different commit messages that explain what each patch is individually doing rather than the same one for all.

Comment 6

[Oleg Zasypkin [:azasypkin]]

Oh, sure. Sorry about that.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cec5c572a516
https://hg.mozilla.org/mozilla-central/rev/6dfa253da690

Comment 8

[Manuela Muntean [Away]]

This issue is still reproducible with latest Nightly on Win 8 64-bit with http://www.google.com/maps/about/explore/ but it's fixed for http://www.haikudeck.com/p/IXPQ9thNi1/10-tips-to-transform-your-presentations#

Any thoughts/suggestions?

Comment 9

[Oleg Zasypkin [:azasypkin]]

(In reply to Manuela Muntean [:Manuela] [QA] from comment #8)
> This issue is still reproducible with latest Nightly on Win 8 64-bit with
> http://www.google.com/maps/about/explore/ but it's fixed for
> http://www.haikudeck.com/p/IXPQ9thNi1/10-tips-to-transform-your-
> presentations#
> 
> Any thoughts/suggestions?

I see that gMaps actually changes url not just url fragment. Not sure whether we can do anything with it, but I'll check.

Comment 10

[Oleg Zasypkin [:azasypkin]]

Yes, looks like we can handle google maps case also. Manuela, can you please file separate bug for handling URL change via pushState/popState/replaceState?

Comment 11

[Manuela Muntean [Away]]

(In reply to Oleg Zasypkin [:azasypkin] from comment #10)
> Yes, looks like we can handle google maps case also. Manuela, can you please
> file separate bug for handling URL change via
> pushState/popState/replaceState?

I've logged bug 976020, so I'm marking this as verified based on comment 8.

Comment 12

[Manuela Muntean [Away]]

Also verified with latest Aurora on Win 8 64-bit using http://www.haikudeck.com/p/IXPQ9thNi1/10-tips-to-transform-your-presentations#