Tiles not working/opening www.metrobrowser.com if firefox not yet running and profilemanager popping up (not auto-use of last profile)

VERIFIED FIXED in Firefox 28

Status

defect
P2
normal
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: aryx, Assigned: TimAbraldes)

Tracking

Trunk
Firefox 29
x86_64
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(firefox28 verified, firefox29 verified)

Details

(Whiteboard: [release28] [defect] p=5)

Attachments

(1 attachment, 1 obsolete attachment)

Firefox Nightly 29.0a1 2014-01-17 on Windows 8.1 Pro 64 bit

I have set in the profile manager of Firefox that it shouldn't automatically use the profile which has been used the last time (my shortcuts provide the profile name).

Opening a tile added to the Metro home page by pinning a page in Firefox Metro launches the profile manager and after selecting the profile, "metrobrowser" can be seen in the location bar, later auto-fixed to www.metrobrowser.com because "metrobrowser" is the first argument of the application shortcut like e.g.

"C:\Program Files (x86)\Browser\Firefox\unstable\central-de\firefox.exe" metrobrowser -url http://www.mozilla.org/en-US/about/
In my case, when I tap on a pinned tile, while I am in desktop mode, the action opens the site in one tab and it also opens www.metrobrowser.com in another tab. I only have a default profile. When I am in metro mode and tap one of these pinned tiles, only the intended site tab opens.
Whiteboard: [testday-20140117] → [testday-20140117] [triage] [defect] p=0
Whiteboard: [testday-20140117] [triage] [defect] p=0 → [release28] [defect] p=0
Whiteboard: [release28] [defect] p=0 → [release28] [defect] p=5
Assignee: nobody → tabraldes
Blocks: metrov1it23
No longer blocks: metrov1backlog
Status: NEW → ASSIGNED
Priority: -- → P2
QA Contact: jbecerra
Posted patch Patch v1 (obsolete) — Splinter Review
The attached patch fixes the issue for newly-created pinned tiles. The patch simply removes any/all instances of "metrobrowser" appearing in our command lines.

I'm guessing that we used to put "metrobrowser" in the command line so the CEH or maybe nsAppRunner would know to launch metro instead of desktop. Our expected/desired behavior is different now; we launch whatever mode was most recently running. Additionally, there doesn't seem to be any code that knows what to do with "metrobrowser" appearing in the command line.

:bbondy - I've heard that you might know how/why we use "metrobrowser" in our command lines. Will there be an issue if we simply don't add that to our command lines anymore?
Flags: needinfo?(netzen)
(In reply to Tim Abraldes [:TimAbraldes] [:tabraldes] from comment #2)
> Created attachment 8364597 [details] [diff] [review]
> Patch v1
> 
> The attached patch fixes the issue for newly-created pinned tiles. The patch
> simply removes any/all instances of "metrobrowser" appearing in our command
> lines.
> 
> I'm guessing that we used to put "metrobrowser" in the command line so the
> CEH or maybe nsAppRunner would know to launch metro instead of desktop. Our
> expected/desired behavior is different now; we launch whatever mode was most
> recently running. Additionally, there doesn't seem to be any code that knows
> what to do with "metrobrowser" appearing in the command line.
> 
> :bbondy - I've heard that you might know how/why we use "metrobrowser" in
> our command lines. Will there be an issue if we simply don't add that to our
> command lines anymore?

I think it was originally used for the reason of front end detection (really early on), but I don't think it's used anymore. So it should be removed.
Flags: needinfo?(netzen)
Comment on attachment 8364597 [details] [diff] [review]
Patch v1

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

Patch looks good but I didn't test it.
Pls make sure to try these things...

Search contract:
 - Search while no browser is open and metro was the last used
 - Search while no browser is open and desktop was the last used
 - Search while metro browser is open
 - Search while desktop browser is open

Activation secondary tile:
 - while no browser is open and metro was the last used
 - while no browser is open and desktop was the last used
 - while metro browser is open
 - while desktop browser is open
Attachment #8364597 - Flags: review+
>  - Search while no browser is open and metro was the last used
>  - Search while no browser is open and desktop was the last used

No search option is available when no browser is open (confirmed in Nightly)

>  - Search while metro browser is open

Searching seemed to have no effect. Opening the error console revealed this error, followed by a handful of others:

[Exception... "Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsICommandLine.resolveURI]"  nsresult: "0x804b000a (NS_ERROR_MALFORMED_URI)"  location: "JS frame :: file:///c:/src/mc2/obj/dist/bin/metro/components/BrowserCLH.js :: resolveURIInternal :: line 25"  data: no]
file:///c:/src/mc2/obj/dist/bin/metro/components/BrowserCLH.js
25

I'll need to investigate and fix this issue before landing.

>  - Search while desktop browser is open

No search option is available when only desktopFx is open (confirmed in Nightly)

> Activation secondary tile:
>  - while no browser is open and metro was the last used

Opened metroFx with my previous session (in accordance with my pref) and also a new tab with the pinned URL. However, the focused/selected tab was the tab from my previous session instead of the pinned URL. I tested with Nightly and this did not happen, so this is another issue introduced by this patch that will have to be addressed before landing.

>  - while no browser is open and desktop was the last used

Opened desktopFx with my previous session (in accordance with my pref) and also a new tab with the pinned URL

>  - while metro browser is open

Switched to metroFx and opened pinned URL in a new tab

>  - while desktop browser is open

Switched to desktopFx and opened pinned URL in a new tab
> >  - Search while metro browser is open
> 
> Searching seemed to have no effect. Opening the error console revealed this
> error, followed by a handful of others:
> 
> [Exception... "Component returned failure code: 0x804b000a
> (NS_ERROR_MALFORMED_URI) [nsICommandLine.resolveURI]"  nsresult: "0x804b000a
> (NS_ERROR_MALFORMED_URI)"  location: "JS frame ::
> file:///c:/src/mc2/obj/dist/bin/metro/components/BrowserCLH.js ::
> resolveURIInternal :: line 25"  data: no]
> file:///c:/src/mc2/obj/dist/bin/metro/components/BrowserCLH.js
> 25
> 
> I'll need to investigate and fix this issue before landing.

Further investigation has revealed that BrowserCLH's "handle" function is not seeing the entire command line; the first argument is being removed, so instead of handling a command line of "-search search_term," we are handling a command line of "search_term." Placing any argument before the "-search" in MetroContracts.cpp fixes the issue (I used the value "dummy" to test).

> > Activation secondary tile:
> >  - while no browser is open and metro was the last used
> 
> Opened metroFx with my previous session (in accordance with my pref) and
> also a new tab with the pinned URL. However, the focused/selected tab was
> the tab from my previous session instead of the pinned URL. I tested with
> Nightly and this did not happen, so this is another issue introduced by this
> patch that will have to be addressed before landing.

I suspect that this has a similar cause as the above: Probably what's happening is that metroFx is not seeing the "-url" part of the command line; it is just seeing the url on the command line.


I haven't yet tracked down where we're stripping the first command line argument. That's the next step of the investigation.
Posted patch Patch v2Splinter Review
This patch addresses the issues I discovered while testing the previous patch. Brian - do you mind taking another look?
Attachment #8364597 - Attachment is obsolete: true
Attachment #8368026 - Flags: review?(netzen)
Attachment #8368026 - Flags: review?(netzen) → review?(jmathies)
Comment on attachment 8368026 [details] [diff] [review]
Patch v2

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

Didn't see any problem with secondary tiles with both the desktop and metro browser running.
Attachment #8368026 - Flags: review?(jmathies) → review+
Landed in fx-team:
  https://hg.mozilla.org/integration/fx-team/rev/8e90d7b17aa5
Whiteboard: [release28] [defect] p=5 → [fixed-in-fx-team][release28] [defect] p=5
https://hg.mozilla.org/mozilla-central/rev/8e90d7b17aa5
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team][release28] [defect] p=5 → [release28] [defect] p=5
Target Milestone: --- → Firefox 29
To land on Aurora plz
Keywords: checkin-needed
Whiteboard: [release28] [defect] p=5 → [a=metro-only][release28] [defect] p=5
https://hg.mozilla.org/releases/mozilla-aurora/rev/40473fad5b78
Keywords: checkin-needed
Whiteboard: [a=metro-only][release28] [defect] p=5 → [release28] [defect] p=5
Verified as fixed, for iteration #23, with latest Beta (28 beta 2) and latest Aurora 29.0a2 on Win 8.1 64-bit.
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.