768950 - Port |Bug 763171 - Display received tabs|

Status: RESOLVED FIXED

(SeaMonkey :: Sync UI, defect)

Description

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch

Like Firefox, we should support the displayURI Sync command for receiving individual links to be opened in a tab.

NB: The issuing side is currently only supported by mobile Firefox. Will attach a test case (e.g. to be executed in Execute JS from xsidebar.mozdev.org) in a minute.

Comment 1

[Jens Hatlak (:InvisibleSmiley)]

Attachment: test case

The attached test case is based on test_clients_engine.js. It outputs to the Error Console for verification purposes only.

Comment 2

[[e-mail]]

Comment on attachment 637171 [details] [diff] [review]
patch

This fires once per browser window, which would be bad any time when we didn't have exactly one browser window open. Sadly I can't find a good place to put some code to handle the case when we have no browser windows open. (The best I can find would be similar to the way the tasksOverlay code passes a dummy command line to nsBrowserContentHandler, but in this case you would have to use a command which then passes the desired URL to handURIToExistingBrowser.)

Comment 3

[Gregory Szorc [:gps]]

There is a new patch on the original bug that fixes the multi-window issue.

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v2 [Checkin: Comment 9]

Thanks Neil for catching the multi/non-window cases. The attached should take care of both issues.

Gavin raised the question on the base bug whether the new tab should be focused. After some thinking my answer is Yes: This command is currently only used for actively sending a tab from a mobile device to a computer, and I think that in that particular case the user's will and expectancy is that the tab is displayed on the computer and raised to the foreground.

Comment 5

[Gregory Szorc [:gps]]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #4)
> Gavin raised the question on the base bug whether the new tab should be
> focused. After some thinking my answer is Yes: This command is currently
> only used for actively sending a tab from a mobile device to a computer, and
> I think that in that particular case the user's will and expectancy is that
> the tab is displayed on the computer and raised to the foreground.

UX explicitly requested that the tab be opened in the background. You may also wish to carry forward the comment in my patch that states why.

Comment 6

[[e-mail]]

Comment on attachment 638006 [details] [diff] [review]
patch v2 [Checkin: Comment 9]

>+      if (mostRecentBrowserWindow) {
>+        mostRecentBrowserWindow.getBrowser().addTab(url, { focusNewTab: true });
[I wonder whether we should raise the window too, i.e. mostRecentBrowserWindow.content.focus();]

>+        Services.ww.openWindow(null, "chrome://navigator/content/navigator.xul", "_blank",
Should check the browser.chromeURL pref; all of our other callers do.
(Followup bug if you prefer.)

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 638006 [details] [diff] [review]
patch v2 [Checkin: Comment 9]

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

Per UX, the tab is supposed to open in the background. This does not violate the principle of least surprise. Having the browser do unexpected things automatically is not good.

That being said, this whole feature is currently a minor exception because Fennec allows sending tabs to clients that don't support displaying tabs because of a limitation in Sync. UX has stated that having the browser do something somewhat unexpected in this case is a lesser evil than having the tab disappear into the ether.

We (Sync) are working on a better long-term solution. See also bug 769744.

See https://hg.mozilla.org/services/services-central/rev/9d0bf5dddfc4 for UX-approved implementation, consistent with Firefox.

Comment 8

[[e-mail]]

(In reply to Gregory Szorc from comment #7)
> Per UX, the tab is supposed to open in the background.
It is legitimate for SeaMonkey to have a different UX to Firefox. For instance we could follow up by adding a preference to choose whether to focus or not.

> Having the browser do unexpected things automatically is not good.
Well, that depends on how you define "automatic".

Comment 9

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 638006 [details] [diff] [review]
patch v2 [Checkin: Comment 9]

http://hg.mozilla.org/comm-central/rev/4e1ef6937c5c
(with comment 6 addressed)