Effectively, this undoes a lot of the work in Bug 1828669, and thus we will have to reopen that bug. This does preserve the de-duping for //exact// URL matches, but not base url + title.
One thing I noticed is that the `page-title-changed` handler operates at O(n) complexity, which probably explains the hangs that Daniel was seeing. We will have to rethink this in terms of de-duping, but for now, I have improved this complexity by maintaining a cache of url -> visit.
This also addresses some of the review comments left by @Standard8 on the [[ https://phabricator.services.mozilla.com/D185315 | original patch ]].
Differential Revision: https://phabricator.services.mozilla.com/D186580
* Add a browser.sessionstore.closedTabsFromClosedWindows pref and consult it when building recently-closed tab lists in firefoxview-next
* Add a _resolveClosedDataSource to SessionStore which allows us to find the window state and _closedTabs given a DOMWindow, a window closedId, or a window session store Id.
* Carry window closedId and source window Id into tab items when building recently-closed tab lists in firefoxview-next. This gives us the necessary context when we need to undoClose or forget a tab item in the list.
* Add a getClosed TabCount and TabData for Closed Windows method to SessionStore
* Modify undoCloseTab and forgetClosedTab to accept source params we can resolve to window state data using _resolveClosedDataSource
Differential Revision: https://phabricator.services.mozilla.com/D185108
In Firefox View, remove duplicate history visits per container. When sorting by date, a container is a single day. When sorting by site, a container is a single host (which, in this case, effectively means that //every// single visit listed must have a unique URL).
We want to get a bit smarter about determining which visits are "duplicates". This is a bit more involved than simply comparing exact URLs. [This doc](https://docs.google.com/document/d/1t13dAW6yMyxKys2Emthe0tZE1Q9j-CTsjRb2fa-LNL0/edit?usp=sharing) explains in further detail, but the summary is that visits should be considered duplicates **if multiple documents have the same title, but have the same base URL (URL possibly without query (?) or fragment (#))**.
SQL is leveraged a bit to handle the straightforward case, i.e. matching exact URL. Frontend is used to further dedupe by "Base URL" + document title.
Some of the caching in `FirefoxViewPlacesQuery` was replaced in favor of getters which utilize existing caches from the base class. Perhaps there's no need for this extension class, will consider refactoring in the future.
https://treeherder.mozilla.org/jobs?repo=try&revision=d8f618ad7c16f3254e871b53725cf039f85d6bc3
Differential Revision: https://phabricator.services.mozilla.com/D185315
* We already make use of the closedId in RecentlyClosedTabsInView to re-open the tab via SS.undoCloseById so it made sense to do the same for forgetting a closed tab
* In the test setup, one of tabs is opened and closed in a different window. This gives us coverage both for the closed tab list rendering, as well as re-opening and forgeting tabs with different source windows
* Both the forget-by-closed-id methods match undoClosedById's behavior and throw if the closedId doesnt match a closed window or tab
Differential Revision: https://phabricator.services.mozilla.com/D184192
Also disable the Firefox View feature tour to avoid any risk of
regressions. The feature tour code will be removed in a later patch.
It's still present for now for testing purposes.
Differential Revision: https://phabricator.services.mozilla.com/D180927
* Add a default-true pref to provide an escape hatch allowing us to revert to previous behavior
* in which recently-closed tabs are per-window,
* and undoing closed tabs restores them to the window they were closed from.
* Ensure we set the pref for tests which depend on its value
* Add some spot-checks in tests with the pref off
Differential Revision: https://phabricator.services.mozilla.com/D179574
* Menu Bar History menu recently-closed tab items includes closed tabs from all currently-open windows
* Toolbar/Appmenu history menu recently-closed tabs list includes closed tabs from all currently-open windows
* Firefox view recently-closed tab list includes closed tabs from all currently-open windows
* All recently-closed tab menu/items re-open in the current window
* Re-open all tabs menu item re-opens all tabs into the current window
* Ensure we filter out tabs without any useful state in firefox-view
* Add a target window argument to undoCloseTab and undoCloseById
* undoCloseTab will remove the tab data from the source window collection and re-open the tab into the target window
* Add an options argument to SessionStore.getWindows to get all private or non-private windows
* Add a getWindowForTabClosedId method on SessionStore, allowing look-up of the window associated with a closed tab
* Ensure recently-closed tab lists only include tabs from non-private windows when attached (i.e. opened from) a non-private window. And vice-versa.
* Update the sessionstore closed tab tests to assert on the new behavior
* Update the browser.sessions.restore implementation to always find and pass the source window when restoring a closed tab
* sessions.restore should always restore closed tabs to the source window as there's no implicit top or current window in the API context
Differential Revision: https://phabricator.services.mozilla.com/D174501
* Add a default-true pref to provide an escape hatch allowing us to revert to previous behavior
* in which recently-closed tabs are per-window,
* and undoing closed tabs restores them to the window they were closed from.
* Ensure we set the pref for tests which depend on its value
* Add some spot-checks in tests with the pref off
Differential Revision: https://phabricator.services.mozilla.com/D179574
* Menu Bar History menu recently-closed tab items includes closed tabs from all currently-open windows
* Toolbar/Appmenu history menu recently-closed tabs list includes closed tabs from all currently-open windows
* Firefox view recently-closed tab list includes closed tabs from all currently-open windows
* All recently-closed tab menu/items re-open in the current window
* Re-open all tabs menu item re-opens all tabs into the current window
* Ensure we filter out tabs without any useful state in firefox-view
* Add a target window argument to undoCloseTab and undoCloseById
* undoCloseTab will remove the tab data from the source window collection and re-open the tab into the target window
* Add an options argument to SessionStore.getWindows to get all private or non-private windows
* Add a getWindowForTabClosedId method on SessionStore, allowing look-up of the window associated with a closed tab
* Ensure recently-closed tab lists only include tabs from non-private windows when attached (i.e. opened from) a non-private window. And vice-versa.
* Update the sessionstore closed tab tests to assert on the new behavior
* Update the browser.sessions.restore implementation to always find and pass the source window when restoring a closed tab
* sessions.restore should always restore closed tabs to the source window as there's no implicit top or current window in the API context
Differential Revision: https://phabricator.services.mozilla.com/D174501
- As closed tabs will change to mean closed tabs from all windows, rename these functions to make
changes in later patches clearer when we mean closed tabs from this window specifically, or closed
tabs for all private/non-private windows
Differential Revision: https://phabricator.services.mozilla.com/D177849
Add logic to apply theme colors to Feature Callout based on where it's
going to show. We can use in-content CSS properties for Firefox View and
other themed system pages, but not for PDF.js, nor for any callouts we
might show in the browser chrome in the future. For the browser chrome
in general, we can use the lightweight theme properties directly, in the
same way the chrome frontend does. But PDF.js is a special case, since
although it exists in the chrome, it's meant to appear like it's in the
PDF.js viewer. And the PDF.js viewer has its own theme totally
independent of everything else. So this dynamically applies themes from
different sources.
This also fixes the bug where the PDF.js color scheme could mismatch the
PDF.js viewer if the browser theme and system color scheme don't match,
e.g. where system color scheme is light but a dark theme is installed,
or vice versa. For PDF.js specifically, we can use the
-moz-content-prefers-color-scheme media query to follow the color scheme
as it exists in the PDF.js viewer page instead of the color scheme in
the chrome window where the Feature Callout actually exists.
It also adds or modifies some colors that were previously missing or
different from the prototype, fixes the illegibility of buttons in HCM
and forced colors mode, and makes some other minor color changes.
Differential Revision: https://phabricator.services.mozilla.com/D173088
Add logic to apply theme colors to Feature Callout based on where it's
going to show. We can use in-content CSS properties for Firefox View and
other themed system pages, but not for PDF.js, nor for any callouts we
might show in the browser chrome in the future. For the browser chrome
in general, we can use the lightweight theme properties directly, in the
same way the chrome frontend does. But PDF.js is a special case, since
although it exists in the chrome, it's meant to appear like it's in the
PDF.js viewer. And the PDF.js viewer has its own theme totally
independent of everything else. So this dynamically applies themes from
different sources.
This also fixes the bug where the PDF.js color scheme could mismatch the
PDF.js viewer if the browser theme and system color scheme don't match,
e.g. where system color scheme is light but a dark theme is installed,
or vice versa. For PDF.js specifically, we can use the
-moz-content-prefers-color-scheme media query to follow the color scheme
as it exists in the PDF.js viewer page instead of the color scheme in
the chrome window where the Feature Callout actually exists.
It also adds or modifies some colors that were previously missing or
different from the prototype, fixes the illegibility of buttons in HCM
and forced colors mode, and makes some other minor color changes.
Differential Revision: https://phabricator.services.mozilla.com/D173088
* Keep track of the tab-pickup-container views in TabsSetupFlowManager and their visibility
* Fix visibilitychange handling in tab-pickup-container and add some tests
* Capture a timestamp when a device has been added and there are 0 tabs to show, with at least one visible tab-pickup-container view
* Record telemetry when there are > 0 tabs in these conditions
* Small change to rename the `_waitingForTabs` internal tracking property to `_waitingForNextTabSync` to better clarify its use and meaning
* Use a consistent pattern in some of the existing tests with how we mock SyncedTabs.getRecentTabs
Differential Revision: https://phabricator.services.mozilla.com/D170526
* Keep track of the tab-pickup-container views in TabsSetupFlowManager and their visibility
* Fix visibilitychange handling in tab-pickup-container and add some tests
* Capture a timestamp when a device has been added and there are 0 tabs to show, with at least one visible tab-pickup-container view
* Record telemetry when there are > 0 tabs in these conditions
Differential Revision: https://phabricator.services.mozilla.com/D170526
When called with isLink=true, _getDragTargetTab returns null if the
pointer is around the edges of the tab. This is useful to decide whether
drag-and-drop should create a new tab, or reuse an existing one.
The problem was that _getDropIndex, used when creating a new tab, would
therefore always get a null tab on dragover events, and then fall back
to iterating all tabs sequentially until it would find the right index,
with an expensive getBoundingClientRect() for each tab.
So this patch:
- Renames unclear isLink to a more meaningful ignoreTabSides
- Makes _getDragTargetTab use the native Element#closest instead of
iterating the ancestors manually in JS.
- Makes _getDropIndex always pass ignoreTabSides=false
- Refactors _getDropIndex to never iterate tabs.
- Adds .tab-drop-indicator{pointer-events:none}. This is needed so that
the indicator doesn't become the event target, we want to get the tab
behind it.
Differential Revision: https://phabricator.services.mozilla.com/D166125
* Keep track of the tab-pickup-container views in TabsSetupFlowManager and their visibility
* Fix visibilitychange handling in tab-pickup-container and add some tests
* Capture a timestamp when a device has been added and there are 0 tabs to show, with at least one visible tab-pickup-container view
* Record telemetry when there are > 0 tabs in these conditions
Differential Revision: https://phabricator.services.mozilla.com/D170526
Set up event handlers so that pressing Escape dismisses Feature
Callouts. If an interactive element outside of the Callout is focused,
then the Escape key will not be consumed. Also consolidate all the event
handlers into a single switch statement so we won't need to continue
adding more callback bindings (they were only necessary before
encapsulation was implemented). Also change the names of a couple
formerly pseudo-private methods that we're now referencing externally.
Differential Revision: https://phabricator.services.mozilla.com/D168307
* And synthesize clicks on the fxview button to ensure we're getting the correct window focus and visibility while asserting on the state
* Ensure the lastTabFetch pref is reset at the top of the tests, and in each test
Differential Revision: https://phabricator.services.mozilla.com/D167623
* Create a sharable module for the core Firefox-View test helpers
* Adjust firefoxview's tests (head.js) to use the helper module
* Adjust messaging-system tests to use the helper module where these helpers were temporarily duplicated
* Adjust newtab tests to use the helper module
Differential Revision: https://phabricator.services.mozilla.com/D167589
Unify the values of "source" and "page" as used in FeatureCallout.sys.mjs:
- Explicitly pass in a value for "page" when instantiating a Feature Callout and use this for the value of "page" when sending Feature Callout telemetry and as the "source" when making calls to `sendTriggerMessage`. This avoids the risk of including non-about: page URLs or PDF file extensions in our telemetry.
- Set the value of "page" in an HTML data attribute that can be accessed for use in about:welcome telemetry for Spotlight and Feature Callouts.
- Update references to the page value previously used as the page/source for telemetry from `about:firefoxview` Feature Callouts from "firefoxview" to "about:firefoxview"
- Pass the token "chrome" when creating a callout from the browser chrome and update references to the source in PDF.js messages' targeting
- Update the page value expected in automated tests as needed
Differential Revision: https://phabricator.services.mozilla.com/D165910
Changed the the section subtitles to heading tags so that VO will recognize them as part of the heading and not a separate clickable element.
Differential Revision: https://phabricator.services.mozilla.com/D165217
There are two things I've added here:
- The observers for when FxA devices are connected/disconnected were not added/removed as part of this update: https://phabricator.services.mozilla.com/D153069.
- When a mobile device is the only synced device beyond the current one (desktop) and you remove the current device (desktop), then sign back in from Fx View Tab Pickup banner, `fxAccounts.device.recentDeviceList` only returns the mobile device for some reason (possibly due to device cache). This causes our checks for a secondary device to fail (as we now only have access to the mobile device from `recentDeviceList`, and we're assuming the one device we DO have access to is our current device - which is not the case). This is why Tab Pickup was incorrectly displaying the "Connect a mobile device" message. I've added a check at the start of `refreshDevices()` to manually refresh the device list (ignoring device cache) if the `recentDeviceList` doesn't contain a device with `isCurrentDevice` set to `true`. This is really a workaround for the caching stuff going on behind the scenes, but this does seem to fix things from our end.
Differential Revision: https://phabricator.services.mozilla.com/D165960
