1225194 - click on awesome bar item from another device doesn't open URL

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Edwin Wong [:edwong]]

1. on another device, open a tab with 'news' in the title
2. on desktop, type 'news' in awesome bar
3. select row from with the sub title 'Your tab from other device'

actual:
moz-action:remotetab,{"url":"https://www.google.com/search?q=izakayz roku&ie=utf-8&oe=utf-8","deviceName":"Fennec%20Aurora%20on%20Pad","originalUrl":"https://www.google.com/search?q=izakayz%20roku&ie=utf-8&oe=utf-8"}

expected: jumps to URL as expected

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0001-Bug-1225194-hook-up-remotetab-action-to-tabs-from-ot.patch

well, that's embarrassing :) It looks like the code to actually take the action got lost somewhere. This patch fixes that and also adds a test.

Comment 2

[Mark Hammond [:markh] [:mhammond]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c7a8cc5a1e8

Comment 3

[Marco Bonardo [:mak]]

Comment on attachment 8688195 [details] [diff] [review]
0001-Bug-1225194-hook-up-remotetab-action-to-tabs-from-ot.patch

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

::: browser/base/content/test/general/browser_bug1225194-remotetab.js
@@ +11,5 @@
> +  EventUtils.synthesizeKey("VK_RETURN", {});
> +
> +  yield promiseTabLoaded;
> +
> +  Assert.equal(gBrowser.selectedTab.linkedBrowser.currentURI.spec, url, "correct URL loaded");

doesn't look like the test is cleaning up after itself for the next tests. It would probably be better to open a new tab, and close it when done (BrowserTestUtils.withNewTab could be handy)

Comment 4

[Mark Hammond [:markh] [:mhammond]]

(In reply to Marco Bonardo [::mak] from comment #3)
> doesn't look like the test is cleaning up after itself for the next tests.
> It would probably be better to open a new tab, and close it when done
> (BrowserTestUtils.withNewTab could be handy)

It's actually OK because there's no new tab open - the current tab is replaced by the action. The test harness itself resets the 1 remaining tab to about:blank (although I guess I could do that too if you are bothered by that?)

Comment 5

[Marco Bonardo [:mak]]

I was just wondering that testing in a new tab is less likely to "break" future tests making assumptions on the pre existing tab (like session history or such). I don't recall if the harness replaces the first tab before each test.
it's not critical, just that I usually prefer to keep each test well separated.

Comment 6

[Mark Hammond [:markh] [:mhammond]]

OK, I've pushed this with the test creating a new tab, thanks.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/13ccab8d10bb

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/13ccab8d10bb