Also, this checks for strings with nulls, which is nice (though I guess an
uncommon mistake).
Differential Revision: https://phabricator.services.mozilla.com/D28312
--HG--
extra : moz-landing-system : lando
The attributes for an interface should be on the line right before the
interface.
Interface attributes should be separated by spaces.
Clean up some trailing whitespace in widget/.
Differential Revision: https://phabricator.services.mozilla.com/D28234
--HG--
extra : moz-landing-system : lando
The existing implementation of inherited attributes keeps a data
structure in a property on the class, but derived classes were
unintentionally getting that structure from their parent class,
preventing derived classes from declaring a different set of inherited
attributes.
Differential Revision: https://phabricator.services.mozilla.com/D28255
--HG--
extra : moz-landing-system : lando
In certain straight-forward cases where we detect a credit card number being used with password fields we will show a dismissed password manager doorhanger. The user can still choose to save in case the valid credit card number is actually their username or password.
1) If the Luhn checksum matches on the username field (see CreditCard.jsm) AND the password is 3 numerical digits (don't handle 4 for now even though it's used by Visa since there are banks that use 4 digits passwords for online banking still).
2) If the Luhn checksum matches on the password value AND we detect that the type=password field is a credit card field via autocomplete=cc-number.
** We must include the @autocomplete check otherwise sites will abuse this loophole on legit login forms and set autocomplete=cc-number on their password fields to avoid saving.
For both of these cases we should `dismissed:true` doorhanger, rather than not showing one at all, in case there are false-negatives.
Differential Revision: https://phabricator.services.mozilla.com/D25485
--HG--
extra : source : e9be442c871e173a409f3b969f5bcea0e1ae4d71
extra : histedit_source : c942a81512be954abe595fa41ca44c26cd89b0e6
In certain straight-forward cases where we detect a credit card number being used with password fields we will show a dismissed password manager doorhanger. The user can still choose to save in case the valid credit card number is actually their username or password.
1) If the Luhn checksum matches on the username field (see CreditCard.jsm) AND the password is 3 numerical digits (don't handle 4 for now even though it's used by Visa since there are banks that use 4 digits passwords for online banking still).
2) If the Luhn checksum matches on the password value AND we detect that the type=password field is a credit card field via autocomplete=cc-number.
** We must include the @autocomplete check otherwise sites will abuse this loophole on legit login forms and set autocomplete=cc-number on their password fields to avoid saving.
For both of these cases we should `dismissed:true` doorhanger, rather than not showing one at all, in case there are false-negatives.
Differential Revision: https://phabricator.services.mozilla.com/D25485
--HG--
extra : transplant_source : %A9%94_%9A%03%00%A1u%F3%28%C6%00H%16z%8A%8F%D6%18O
This patch adds an ID to ensure that we avoid canceling content JS if the next
page already started loading by the time we're ready to try canceling the JS.
Differential Revision: https://phabricator.services.mozilla.com/D25164
--HG--
extra : moz-landing-system : lando
In this part, we pass along the navigation type (and index for when using the
dropdown on the back/forward buttons). This allows us to check if there's a
top-level load "between" the start and end pages.
The patch might look a bit strange, since we're passing the navigation
operation to two places from RemoteWebNavigation.js (the normal message passing
that existed before this patch and the HangMonitor channel in this patch). This
is primarily to make it easier to stop passing the navigation info along the
HangMonitor channel once session history is uplifted into the parent process.
At that point, the check for whether there's a top-level load could happen in
TabParent (I think).
Differential Revision: https://phabricator.services.mozilla.com/D23090
--HG--
extra : moz-landing-system : lando
This patch passes a message through the HangMonitor channel when navigating
through history to cancel content JS that could hang the chrome JS in the
content process responsible for history navigation. If the content JS is
actually canceled, this also disables the BF cache for the current page, since
it could end up in an inconsistent state due to the JS cancellation.
Differential Revision: https://phabricator.services.mozilla.com/D23089
--HG--
extra : moz-landing-system : lando
Adding telemetry for the amount of traffic transfer over the captive portal. Also moving some telemetry for trackers to the same place where our old telemetry is, so that they are all on the same place.
Differential Revision: https://phabricator.services.mozilla.com/D27375
--HG--
extra : moz-landing-system : lando
This will provide telemetry so the affect of BITS update downloads with nsIIncrementalDownload update downloads can be compared.
Since a scalar can only be recorded once per session there are two scalars for each data point.
The update.startup telemetry is used to record values when an update finished during startup which prevents it from being recorded multiple times during a session.
The update.session telemetry is used to record values when an update finished during a session and there is a guard that prevent it from being recorded multiple times during a session.
All values for update.startup or update.session are submitted at the same time to simplify querying the values.
The tests only cover nsIIncrementalDownload since we don't have the ability to test BITS yet.
There is also some minor test cleanup.
Differential Revision: https://phabricator.services.mozilla.com/D27838
--HG--
extra : moz-landing-system : lando
Bug 1495072 uncovered a race in the webextension persistent listener logic
where the Promise returned by a listener that is not re-registered during
extension startup may never resolve. When this occurs with a blocking
webRequest listener, content loads just hang forever.
Fix this by forcing primed listeners to reject is they are invoked after
the background page has started.
Differential Revision: https://phabricator.services.mozilla.com/D27942
--HG--
extra : rebase_source : f6179911291348c6a0ed99609bbc5fe5526eaa74
This will provide telemetry so the affect of BITS update downloads with nsIIncrementalDownload update downloads can be compared.
Since a scalar can only be recorded once per session there are two scalars for each data point.
The update.startup telemetry is used to record values when an update finished during startup which prevents it from being recorded multiple times during a session.
The update.session telemetry is used to record values when an update finished during a session and there is a guard that prevent it from being recorded multiple times during a session.
All values for update.startup or update.session are submitted at the same time to simplify querying the values.
The tests only cover nsIIncrementalDownload since we don't have the ability to test BITS yet.
There is also some minor test cleanup.
Differential Revision: https://phabricator.services.mozilla.com/D27838
--HG--
extra : moz-landing-system : lando
This provides a flipped data structure based on the provided inheritedAttributes,
which looks like:
Object<selector, attrs_to_inherit_comma_separated>
To one that looks like:
Object<attr, Array<Array<selector, attr_to_inherit>>
This should improve performance because:
1) We only fetch element at connectedCallback that actually will have attributes inherited.
2) When an attribute changes we can quickly inherit only that one.
Differential Revision: https://phabricator.services.mozilla.com/D27801
--HG--
extra : moz-landing-system : lando
The "SwapDocShells" event should be deferred until "EndSwapDocShells".
Otherwise the event MessageManagerProxy may swap the event listeners
twice, and end up having the listeners on the incorrect message manager.
Differential Revision: https://phabricator.services.mozilla.com/D27295
--HG--
extra : moz-landing-system : lando
For bug 726781, the Windows installer was patched to begin creating a special
registry key when installing an ESR build, to provide a convenient indication
that the product that's installed is an ESR version of the product.
This key contains only the version number of the application being installed;
it is separate from the keys that are always created, for all types of
builds, and that contain the installation path, etc.
During an uninstall or a paveover install, the registry is cleaned by looking
for any keys which contain the path to the application being uninstalled and
removing those; the RegCleanMain function handles this, and for non-ESR builds
it works well. However, there is nothing that tries to remove or update the
special ESR key when an ESR build is being uninstalled or paved over with a
non-ESR build. This patch adds that code to RegCleanMain.
Differential Revision: https://phabricator.services.mozilla.com/D26625
--HG--
extra : moz-landing-system : lando
We don't want spurious net connections, so we should mock the result.
In addition, we need to mock it to a specific non-North-American region to
ensure we don't cause an extra change to browser.search.region which may split
the subsession in the middle of our tests.
Differential Revision: https://phabricator.services.mozilla.com/D28059
--HG--
extra : moz-landing-system : lando
Pausing isn't implemented for BITS and this UI will be removed entirely in the future.
Differential Revision: https://phabricator.services.mozilla.com/D27847
--HG--
extra : moz-landing-system : lando
On startup we record the size and modified time of the profile lists. If
changed we refuse to flush any new changes to disk. Also adds a getter to check
if they've changed so the UI can do something sensible.
All attempts to flush are now checked for success. In some cases in early
startup the failure mode isn't great, we just quit startup. The assumption
though is that it's extremely unlikely that the files will have changed on disk
in the time between when they are read and when profile selection occurs, likely
less than a second later.
The profile reset flow is changed to only delete the old profile and flush once
all the migration has completed, so if something fails the user gets back to
their old profile.
In testing I ended up having to fix bug 1522584 so background file deletions on
a background thread are safer.
I haven't implemented any UI tests right now since making modifications to the
profiles means modifying the actual user's profiles which I'm not keen to do.
See bug 1539868.
Differential Revision: https://phabricator.services.mozilla.com/D25278
--HG--
extra : moz-landing-system : lando
There's a readonly shorcut, which prevents us altogether to register the
mutation observer which would notify us of that attribute changing.
Move the readonly check further down to StartControllingInput, so that we
register the mutation observer properly.
Differential Revision: https://phabricator.services.mozilla.com/D27883
--HG--
extra : moz-landing-system : lando
Almost none of the prompts that we are currently showing still get dismissed, which messes up our
measurements in this probe. Most of them are persistent now, which means that we should record when
they get removed instead of dismissed to receive meaningful data. This patch does that.
Differential Revision: https://phabricator.services.mozilla.com/D26944
--HG--
extra : moz-landing-system : lando
- `Array.map` becomes `Array.from`
- Array copying via `Array.slice` becomes `Array.from`.
- `Array.forEach` that did not rely on closures becomes `for`-`of` loops.
- Anything else: `Array.X` becomes `Array.prototype.X`.
Complex cases:
dom/bindings/test/TestInterfaceJS.js and
dom/bindings/test/test_exception_options_from_jsimplemented.html
use `Array.indexOf` to generate an error with a specific error message.
Switched to `Array.prototype.forEach` to generate the same error.
js/src/jit-test/tests/basic/exception-column-number.js
In this test `Array.indexOf()` is used to generate an error. Since the
exact message doesn't matter, I switched to `Array.from()`.
Intentionally not changed:
editor/libeditor/tests/browserscope/lib/richtext/richtext/js/range.js
Did not modify because this is 3rd-party code and the code uses
feature detection as a fall back when Array generics are not used.
testing/talos/talos/tests/dromaeo/lib/mootools.js
Did not modify because mootools adds the `Array.slice` method to the
`Array` object.
Not changed because they check the implementation of Array generics:
js/src/jit-test/tests/basic/arrayNatives.js
js/src/jit-test/tests/basic/bug563243.js
js/src/jit-test/tests/basic/bug618853.js
js/src/jit-test/tests/basic/bug830967.js
js/src/jit-test/tests/jaeger/recompile/bug656753.js
js/src/jit-test/tests/self-hosting/alternate-static-and-instance-array-extras.js
js/src/tests/non262/Array/generics.js
js/src/tests/non262/Array/regress-415540.js
js/src/tests/non262/extensions/regress-355497.js
js/src/tests/non262/extensions/typedarray-set-neutering.js
Depends on D27802
Differential Revision: https://phabricator.services.mozilla.com/D27803
--HG--
extra : moz-landing-system : lando
Moved changes to the non-forked part of breakpad living under
toolkit/crashreporter/google-breakpad into separate patches that are applied
by update-breakpad.sh when synchronizing with upstream breakpad. Because we
landed the commits directly to the sources every time we called
update-breakpad.sh those changes would be reverted.
Differential Revision: https://phabricator.services.mozilla.com/D27682
--HG--
extra : moz-landing-system : lando
On startup we record the size and modified time of the profile lists. If
changed we refuse to flush any new changes to disk. Also adds a getter to check
if they've changed so the UI can do something sensible.
All attempts to flush are now checked for success. In some cases in early
startup the failure mode isn't great, we just quit startup. The assumption
though is that it's extremely unlikely that the files will have changed on disk
in the time between when they are read and when profile selection occurs, likely
less than a second later.
The profile reset flow is changed to only delete the old profile and flush once
all the migration has completed, so if something fails the user gets back to
their old profile.
In testing I ended up having to fix bug 1522584 so background file deletions on
a background thread are safer.
I haven't implemented any UI tests right now since making modifications to the
profiles means modifying the actual user's profiles which I'm not keen to do.
See bug 1539868.
Differential Revision: https://phabricator.services.mozilla.com/D25278
--HG--
extra : moz-landing-system : lando
The previous commit disabled the remote agent by flipping the
remote.enabled preference to false. That prevented the remote
agent from initialising or being included in the --help message.
This patch implies --enable-cdp in the default Firefox build on Firefox
Nightly. Firefox for Android is not supported. This will cause
builds to include the remote agent component that lives under remote/.
Since the remote agent is disabled by default, users will first
have to set the remote.enabled preference to true in order to use it.
If you wish to explicitly opt out of including the remote agent
when building Firefox, you may do so by using the --disable-cdp
build flag in your mozconfig:
ac_add_options --disable-cdp
Differential Revision: https://phabricator.services.mozilla.com/D27540
--HG--
extra : moz-landing-system : lando
`Async.jankYielder` is known to, unfortunately, cause jank by creating a lot of
immediately resolved promises that must be then GCed. For a collection of 50
items, it will create 50 promises and 49 of them will immediately resolve.
Instead of `Async.jankYielder`, we now have `Async.yieldState`, which simply
keeps track of whether or not the caller should yield to the event loop. Two
higher level looping constructs are built on top of it:
* `Async.yieldingIterator`, which has been rewritten to not create extraneous
promises; and
* `Async.yieldingForEach`, which is a replacement for awaiting
`Async.jankYielder` in a loop. Instead, it accepts the loop body as a
function.
Each of these can share an instance of an `Async.yieldState`, which allows an
object with multiple loops to yield every N iterations overall, instead of
every N iterations of each loop, which keeps the behaviour of using one
`Async.jankYielders` in multiple places.
Differential Revision: https://phabricator.services.mozilla.com/D26229
--HG--
extra : moz-landing-system : lando
This change allows GeckoView embedders to respond to
`runtime.sendNativeMessage` and `runtime.connectNative` sent from
WebExtensions.
These APIs are available behind the new privileged-only permission
`geckoViewAddons` and are used by GeckoView apps to communicate between content
and the app.
Depends On D22621
Differential Revision: https://phabricator.services.mozilla.com/D22622
--HG--
extra : moz-landing-system : lando
This provides a flipped data structure based on the provided inheritedAttributes,
which looks like:
Object<selector, attrs_to_inherit_comma_separated>
To one that looks like:
Object<attr, Array<Array<selector, attr_to_inherit>>
This should improve performance because:
1) We only fetch element at connectedCallback that actually will have attributes inherited.
2) When an attribute changes we can quickly inherit only that one.
Differential Revision: https://phabricator.services.mozilla.com/D27801
--HG--
extra : moz-landing-system : lando
Adding telemetry for the amount of traffic transfer over the captive portal. Also moving some telemetry for trackers to the same place where our old telemetry is, so that they are all on the same place.
Differential Revision: https://phabricator.services.mozilla.com/D27375
--HG--
extra : moz-landing-system : lando
Remove eval() from updates.js
Refactor code to make mapping of pageids to js-objects obsolete
Remove now unnecessary object attributes from wizard pages in updates.xul
Differential Revision: https://phabricator.services.mozilla.com/D25921
--HG--
extra : moz-landing-system : lando
Configure the lmdb-rkv-sys Rust crate to use a smaller MDB_IDL_LOGN size in order to reduce allocations when opening an LMDB environment in read-write mode.
@glandium I adopted the configuration strategy you suggested of creating a "feature" for each reasonable value for the MDB_IDL_LOGN macro. Fortunately, the range of reasonable values is fairly small.
@nanj Based on your evalution in https://github.com/mozilla/lmdb/pull/2, a value of "9" for this macro should aggressively reduce the allocations while still supporting our existing use cases. But I'm open to increasing it, if you think a higher initial value would be preferable.
Differential Revision: https://phabricator.services.mozilla.com/D27559
--HG--
extra : moz-landing-system : lando
This excludes dom/, otherwise the file size is too large for phabricator to handle.
This is an autogenerated commit to handle scripts loading mochitest harness files, in
the simple case where the script src is on the same line as the tag.
This was generated with https://bug1544322.bmoattachments.org/attachment.cgi?id=9058170
using the `--part 2` argument.
Differential Revision: https://phabricator.services.mozilla.com/D27456
--HG--
extra : moz-landing-system : lando
These Histograms were added:
UPDATE_CAN_USE_BITS_EXTERNAL
UPDATE_CAN_USE_BITS_NOTIFY
Used for telemetry indicating whether or not BITS can be used by this system. If BITS cannot be used, the probe will contain data indicating why not.
UPDATE_BITS_RESULT_COMPLETE
UPDATE_BITS_RESULT_PARTIAL
Used for telemetry indicating whether the BITS update download succeeded. If it failed, the probe will contain data indicating how it failed.
This scalar was added:
update.bitshresult
Used to indicate the hresult returned, if any, when a BITS download fails.
Differential Revision: https://phabricator.services.mozilla.com/D25163
--HG--
extra : moz-landing-system : lando
nsUpdateService should use BITS for download. If the BITS download fails, it will fallback to the existing download mechanism (nsIIncrementalDownload).
Differential Revision: https://phabricator.services.mozilla.com/D25162
--HG--
extra : moz-landing-system : lando
This patch introduces an asynchronous XPCOM interface for the bits client added in Bug 1523417
Differential Revision: https://phabricator.services.mozilla.com/D25161
--HG--
extra : moz-landing-system : lando
* Use Services.(ppmm|cpmm).sharedData to make isMasterPasswordSet value available to the content process
* We don't handle runtime enable/disable of MP
Differential Revision: https://phabricator.services.mozilla.com/D26939
--HG--
extra : moz-landing-system : lando
This should not have any major behaviour changes, with the following exceptions:
1. The method for receiving messages from IPC is called `receiveMessage` rather
than `recvAsyncMessage`. This is more consistent with existing code, so
should be OK.
2. Exceptions will be correctly reported when thrown within a callback.
Differential Revision: https://phabricator.services.mozilla.com/D26547
--HG--
extra : moz-landing-system : lando
We need to handle autofilling the first result separately from autofilling results in general (which happens in UrlbarInput.setValueFromResult), so add a new UrlbarInput.autofillFirstResult method. The controller calls it instead of setValueFromResult. I ported the logic from nsAutoCompleteController, as described in the bug.
Other changes are related to the new test for this.
As part of this work, I was interested in learning how awesomebar handles browser_autoFill_typed.js, so I added it to the legacy tests, with a small tweak in the test for awesomebar.
Differential Revision: https://phabricator.services.mozilla.com/D26852
--HG--
extra : moz-landing-system : lando
This also stops the PictureInPictureToggleChild from tracking videos with controls for now.
Depends on D26777
Differential Revision: https://phabricator.services.mozilla.com/D26778
--HG--
extra : moz-landing-system : lando
Remove the tree._columnsDirty property which is causing problems for column reordering in a fresh profile.
Differential Revision: https://phabricator.services.mozilla.com/D25507
--HG--
extra : moz-landing-system : lando
There shouldn't be any need to do this for content processes as
the DLL should already be in the system file cache.
Differential Revision: https://phabricator.services.mozilla.com/D26017
--HG--
extra : moz-landing-system : lando
So, this patch makes all caller of it safe including its arguments unless
they come from other methods.
Differential Revision: https://phabricator.services.mozilla.com/D27225
--HG--
extra : moz-landing-system : lando
This also stops the PictureInPictureToggleChild from tracking videos with controls for now.
Differential Revision: https://phabricator.services.mozilla.com/D26778
--HG--
extra : moz-landing-system : lando
This patch makes `nsDocShell::GetPresShell()` and
`nsDocShell::GetEldestPresShell()` return `mozilla::PresShell*` and
some non-public methods use `mozilla::PresShell*` directly.
Differential Revision: https://phabricator.services.mozilla.com/D26424
--HG--
extra : moz-landing-system : lando
Password manager should not offer to save credit card numbers in certain straight-forward cases.
Differential Revision: https://phabricator.services.mozilla.com/D25485
--HG--
extra : moz-landing-system : lando
`Async.jankYielder` is known to, unfortunately, cause jank by creating a lot of
immediately resolved promises that must be then GCed. For a collection of 50
items, it will create 50 promises and 49 of them will immediately resolve.
Instead of `Async.jankYielder`, we now have `Async.yieldState`, which simply
keeps track of whether or not the caller should yield to the event loop. Two
higher level looping constructs are built on top of it:
* `Async.yieldingIterator`, which has been rewritten to not create extraneous
promises; and
* `Async.yieldingForEach`, which is a replacement for awaiting
`Async.jankYielder` in a loop. Instead, it accepts the loop body as a
function.
Each of these can share an instance of an `Async.yieldState`, which allows an
object with multiple loops to yield every N iterations overall, instead of
every N iterations of each loop, which keeps the behaviour of using one
`Async.jankYielders` in multiple places.
Differential Revision: https://phabricator.services.mozilla.com/D26229
--HG--
extra : moz-landing-system : lando
This includes improvements to the Linux exception handler that will provide
crash addresse for a number of signals by reading the NT_SIGINFO structure and
quites warnings in a number of files. This also removes an unused header.
Differential Revision: https://phabricator.services.mozilla.com/D26488
--HG--
extra : moz-landing-system : lando
There shouldn't be any need to do this for content processes as
the DLL should already be in the system file cache.
Differential Revision: https://phabricator.services.mozilla.com/D26017
--HG--
extra : moz-landing-system : lando
nsBrowserStatusFilter is updated to not filter out STATE_IS_REDIRECTED_DOCUMENT.
The test here is adding a way to have a "login form" do a post to a server script, which then does a 303 redirect. This mimics what some services, including LinkedIn do during this stage.
Differential Revision: https://phabricator.services.mozilla.com/D26969
--HG--
extra : moz-landing-system : lando
After calling Lookup API per table, Safe Browsing outputs too many debug
message for a single URL lookup. Refine the current output.
Differential Revision: https://phabricator.services.mozilla.com/D27066
--HG--
extra : moz-landing-system : lando
`Async.jankYielder` is known to, unfortunately, cause jank by creating a lot of
immediately resolved promises that must be then GCed. For a collection of 50
items, it will create 50 promises and 49 of them will immediately resolve.
Instead of `Async.jankYielder`, we now have `Async.yieldState`, which simply
keeps track of whether or not the caller should yield to the event loop. Two
higher level looping constructs are built on top of it:
* `Async.yieldingIterator`, which has been rewritten to not create extraneous
promises; and
* `Async.yieldingForEach`, which is a replacement for awaiting
`Async.jankYielder` in a loop. Instead, it accepts the loop body as a
function.
Each of these can share an instance of an `Async.yieldState`, which allows an
object with multiple loops to yield every N iterations overall, instead of
every N iterations of each loop, which keeps the behaviour of using one
`Async.jankYielders` in multiple places.
Differential Revision: https://phabricator.services.mozilla.com/D26229
--HG--
extra : moz-landing-system : lando
randomly choose 1% users and their 0.14% page view to measure content blocking.
Differential Revision: https://phabricator.services.mozilla.com/D26130
--HG--
extra : moz-landing-system : lando
We don't need an additional array just for byte reordering, replace
it with in-place processing.
Testcase are modified because the LookupCacheV4::Build API now clears the
input parameter.
Differential Revision: https://phabricator.services.mozilla.com/D26861
--HG--
extra : moz-landing-system : lando
Here is the flow how prefixes are handled during an V4 update:
1. Prefixes are received from Safe Browsing update, stored in ProtocolBuffer
2. Copy the prefixes from ProtocolBuffer to TableUpdate structure
3. Prefixes in TableUpdate are merged with local prefixes (stored in LookupCacheV4)
4. Merged prefixes are processes by PrefixSet to generate the in-memory prefix
set data structure (MakePrefixSet).
In this patch, we free the prefixes stored in TableUpdate right after step3.
This reduces the peak memory used during an update (peak happens in step 4).
Differential Revision: https://phabricator.services.mozilla.com/D26860
--HG--
extra : moz-landing-system : lando
In particular, this enables Marionette in local Fennec builds, which
were the only place it wasn't enabled by default. (Automation builds
all enabled Marionette.) That default is getting in the way of the
Performance Team (and others!) testing GeckoView-based products
easily.
Differential Revision: https://phabricator.services.mozilla.com/D26815
--HG--
extra : moz-landing-system : lando
We also take account those values in the case of `Find in page`.
The corresponding web platform tests will be coming from this PR.
https://github.com/web-platform-tests/wpt/pull/8575
Though some of them will not be passed, the failure reason is not related
to this change, I will take a look when the PR gets merged into mozilla-central.
Differential Revision: https://phabricator.services.mozilla.com/D25915
--HG--
extra : moz-landing-system : lando
In the case where scroll-snap-type is specified for the scroll container, the
scroll-padding is also factored into in ScrollFrameHelper::ComputeScrollSnapInfo
which is called via ScrollFrameHelper::ScrollToWithOrigin. It doesn't double
the scroll-padding value, but it's actually redundant, we should avoid it.
We could separate the functionality of ScrollToWithOrigin, one is to scroll
to a given element, the other is to scroll to a given position. The former will
be used for Element.scrollIntoElement and relevant stuff, the latter will be
used for Element.scrollTo and relevant stuff. That's being said, as of now, we
have still the old scroll snap implementation, so the separation will introduce
complexity, the separation should be done once after the old implementation
removed.
There are 9 call sites of nsIPresShell::ScrollContentIntoView:
nsIPresShell::GoToAnchor
nsIPresShell::ScrollToAnchor
Element::ScrollIntoView
We definitely needs scroll-padding and scroll-margin for these functions.
nsCoreUtils::ScrollTo
This is used for Accesible::ScrollTo which scrolls to a given accesible node,
probably we should behave as what Element::ScrollIntoView does.
Accessible::DispatchClickEvent
Similar to the above, similated various mouse events on a given target node.
PresShell::EventHandler::PrepareToUseCaretPosition
PresShell::EventHandler::GetCurrentItemAndPositionForElement
Both are for context menu, we shouldn't consider scroll-padding and
scroll-margin.
nsFormFillController::SetPopupOpen
This is used for autocompletion popup, we shouldn't consider scroll-padding
and scroll-margin.
nsFocusManager::ScrollIntoView
This is bit unfortunate, we should use scroll-padding and scroll-margin
depending on call site of this function. Bug 1535232 is for this case.
cssom-view/scrollIntoView-scrollPadding.html which has some tests that is
actually testing scroll-padding with scrollIntoView passes with this change.
The reftest in this change is a test case that the browser navigates to an
element with specifying the anchor to the element.
Differential Revision: https://phabricator.services.mozilla.com/D23084
--HG--
extra : moz-landing-system : lando
Emilio Cobos Álvarez
Myk Melez
Andreea Pavel
Razvan Maries
Ciure Andrei
Daniel Varga
Andrew McCreight
Andrew Swan
Gurzau Raul
Robert Helmer
Brindusan Cristian
prathiksha
Matthew Noorenberghe
Dave Townsend
Brian Grinstead
Coroiu Cristina
Robert Strong
Jim Porter
Alexander Surkov
Dragana Damjanovic
Marco Bonardo
Dale Harvey
Dão Gottwald
Noemi Erli
feugy
Arpit
Olli Pettay
Rob Wu
Matt Howell
Chris H-C
Mike Conley
Tim D. Smith
Dorel Luca
Ehsan Akhgari
Mark Banner
Johann Hofmann
monikamaheshwari
Bogdan Tara
Jared Wein
Mike de Boer
Tim Nguyen
Nathan Froyd
Kirk Steuber
Agi Sferro
Richard Marti
Gabriele Svelto
Andreas Tolfsen
Barret Rennie
Luke Crouch
Csoregi Natalia
Sylvestre Ledru
Gijs Kruitbosch
lloan
Jonas Allmann
Felipe Gomes
Luca Greco
Kestrel
Ethan Glasser-Camp
Jared Hirsch
Kartikaya Gupta
Sam Foster
Aaron Klotz
Nika Layzell
Drew Willcoxon
Andrea Marchesini
Magnus Melin
Doug Thayer
Masayuki Nakano
Avery Berninger
Liang-Heng Chen
Cosmin Sabou
Shane Caraveo
Oana Pop Rus
Michael Kaply
dlee
Boris Zbarsky
nidhi
Nico Grunbaum
rdalal
Nick Alexander
Hiroyuki Ikezoe
Mike Hommey