- 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
Check the triggering principal of document loads in TabProgressListener's onLocationChange event,
and clear the related tabs if load was triggered by the system principal (typed in the url bar / bookmarks / history, etc.) to prevent grouping unrelated tabs.
Differential Revision: https://phabricator.services.mozilla.com/D178373
Not sure if this is worth it, your call. But I wrote this so I figured
someone debugging this code might find it useful.
Depends on D172033
Differential Revision: https://phabricator.services.mozilla.com/D172034
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
Either of these changes (ie dropping the setTabState call for batch restored
tabs, or ensuring the restoreTabs code correctly fills its array with dummy
entries) is sufficient here. I chose to do both because I think in both cases
the brokenness is not limited to this scenario or the issues at hand.
Specifically, the setTabState call was added in bug 1521346 to deal with
moved lazy tabs, but is now being invoked for session restore because of
the batchInsertingTabs optimization work. It doesn't actually need to be,
as far as I can tell, and the lacking _tPos in this case (because we don't
insert the tab into the tabstrip a few lines above) is what breaks things
inside _ensureNoNullsInTabDataList. Note that this _already_ was breaking
things in restoreTab(), which would assign into tabs[undefined] on the
window state object, so just dropping the call seemed better than wallpapering
the absence of _tPos.
The restoreTabs code, pre-patch, calls _ensureNoNullsInTabDataList but that
will never do anything, because right before calling it we change the array
length, so maxPos was always smaller than the size of the list. This meant
we still had empty slots in the array, which was also causing confusion down
the line.
I added the explicit exception for the broken _tPos in restoreTab so that we
notice any future issues with this more quickly. Doing so without any of the
other fixes broke the pre-existing browser_586068-apptabs.js test, so
hopefully that will catch any future changes that break the code's assumptions.
Differential Revision: https://phabricator.services.mozilla.com/D173070
I'm removing the apptab bits completely in one of the later commits,
but trying to keep this modular so it's easy to figure out regressions,
should there be any.
Differential Revision: https://phabricator.services.mozilla.com/D171411
The unnecessary tabbrowser-tab-tooltip is dropped, as it's the same single variable references in all locales. Also, setting the title as the label directly avoids a minuscule but observable delay that's the source of this issue.
Differential Revision: https://phabricator.services.mozilla.com/D171103
This moves the somewhat out-of-place `_loadURI` method and its dependencies
into tabbrowser, and deals with receiving either a string or a URI.
Depends on D168391
Differential Revision: https://phabricator.services.mozilla.com/D168392
This moves the somewhat out-of-place `_loadURI` method and its dependencies
into tabbrowser, and deals with receiving either a string or a URI.
Depends on D168391
Differential Revision: https://phabricator.services.mozilla.com/D168392
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
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
