forked from web-platform-tests/wpt
-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bump peter-evans/create-pull-request from v2 to v3.8.2 #1
Closed
dependabot
wants to merge
1
commit into
master
from
dependabot/github_actions/peter-evans/create-pull-request-v3.8.2
Closed
Bump peter-evans/create-pull-request from v2 to v3.8.2 #1
dependabot
wants to merge
1
commit into
master
from
dependabot/github_actions/peter-evans/create-pull-request-v3.8.2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bumps [peter-evans/create-pull-request](https://github.com/peter-evans/create-pull-request) from v2 to v3.8.2. - [Release notes](https://github.com/peter-evans/create-pull-request/releases) - [Commits](peter-evans/create-pull-request@v2...052fc72) Signed-off-by: dependabot[bot] <support@github.com>
dependabot
bot
added
dependencies
Pull requests that update a dependency file
github_actions
Pull requests that update Github_actions code
labels
Mar 16, 2021
mrego
pushed a commit
that referenced
this pull request
Mar 18, 2021
Currently, the CheckCanStartAnimationOnCompositor is called twice for composite background-color animation, once during paint time and once during the compositing stage. The reason is that we need the decision during paint and compositing consistent. That is, if the paint stage says the background color must be paint on the main thread, then compositing stage has to agree with that, and vice versa. However, this is dangerous because between the paint and compositing stage, things could change, especially the PaintArtifactCompositor, which is used by the CheckCanStartAnimationOnCompositor. For example, it could happen that at paint time we have not produced / updated the property nodes for the current frame and we can make decision based on what was composited on the previous frame. Then at Precommit we have potentially updated / added / removed property tree nodes. In this case, the return value of CheckCanStartAnimationOnCompositor can be different, as a result, the background color animation won't run correctly. The reason we needed to know whether the animation could be composited here is that we didn't have a way to paint the background color off the main thread. More specifically, the BackgroundColorPaintWorklet::Paint() function can paint the background color only if the animation is running on the compositor thread. This CL makes following changes: 1. Make the BackgroundColorPaintWorklet::Paint() have the ability to paint the background color even if the animation is running on the main thread. The function needs two things: the current progress of the animation and the artifacts about the animation. So all we need is just getting the progress when the animation is running on the main thread. 2. With #1 being done, we no longer need to call the CheckCanStartAnimationOnCompositor during the paint step. As a result, whether or not the animation can be running on the compositor thread is solely the decision during the compositing stage. This is much safer than the current code, because we no longer need to make a compositing decision during the paint stage. We don't need to add any new tests because we already have sufficient layout tests for background color animation being run on the compositor as well as on the main thread. As long as all tests pass, this should be safe. The main benefit of this change is that the code is now more robust, meaning that we don't need to worry about the decision made by the paint and compositing stage being different. This change is also a performance win because we no longer need to call the CheckCanStartAnimationOnCompositor twice. Bug: 1185272, 1182261 Change-Id: Ie072714fd1d05e6537e05cad45ad1da99e20125b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2740697 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#863622}
Looks like peter-evans/create-pull-request is up-to-date now, so this is no longer needed. |
dependabot
bot
deleted the
dependabot/github_actions/peter-evans/create-pull-request-v3.8.2
branch
April 20, 2021 13:08
mrego
pushed a commit
that referenced
this pull request
Apr 27, 2021
…eb-platform-tests#28617) Subresource Web Bundles. The problem is: when Web Bundle fetching fails due to a network error, Subresource fetch doesn't fail forever. One such case (subresource-loading-cors-error test) was timing out previously but passes successfully with this change. This CL also adds 2 WPT tests: 1. subresource-loading-network-error.https.tentative.sub.html 2. subresource-loading-web-bundle-fetch-failed.https.tentative.html Test #1 is a scenario with a different network error than the CORS one, but with the same issue of subresource fetching timing out without the change. It passes successfully after the change. Test #2 is a scenario with a Web bundle not found error, which is not directly influenced by the code added in this CL, but it expands the test coverage which was found to be lacking the error cases before. Bug: 1168449 Change-Id: Ia3abb967e36274becc86e317bc51b1272d3ae679 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2826001 Reviewed-by: Tsuyoshi Horo <horo@chromium.org> Reviewed-by: Hayato Ito <hayato@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Miras Myrzakerey <myrzakereyms@google.com> Cr-Commit-Position: refs/heads/master@{#875532} Co-authored-by: Miras Myrzakerey <myrzakereyms@google.com>
mrego
pushed a commit
that referenced
this pull request
Jul 19, 2021
1. Only process ChildrenChanged() for the included root of a change. For example, if a <div id="root" style="display:none"> will be included because it is a potential relation target. If descendants change, the only ChildrenChanged() necessary to process is on #root. 2. Share common code for detaching a node and queuing up the appropriate children changes. This simplifies ProcessInvalidatedObjects() by removing one of the inner loops, and enables a follow-up CL to remove the outer loop as well. #1 results in a massive speedup for display none toggles. In combination with other recent changes in DetachAndRemoveFromChildrenOfAncestors(), is 7x faster for many-nodes-toggle-display-none in perf_tests . This change alone accounts for about half of the overall improvement. Follow-ups: - Restore lifecycle check by processing deferred children changes via nodes_with_pending_children_changed_ and not queuing via the traditional mechanism. While doing this, look for opportunities to consolidate more children changed events. - Remove outer loop from ProcessInvalidatedObjects(). Bug: None Change-Id: I80466fda792cd0ca6dd051065a42ba702e4cc8b1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2946971 Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Commit-Queue: Aaron Leventhal <aleventhal@chromium.org> Cr-Commit-Position: refs/heads/master@{#891343}
mrego
pushed a commit
that referenced
this pull request
Jul 19, 2021
1. Use GetWithoutInvalidation() instead of Get() in DCHECKs. We should never call Get() inside of a DCHECK(), because this can lead to a different code path depending on whether DCHECKs are enabled. 2. Get() should not cause immediate side effects. At most, it should queue up an invalidation for later processing. Fixing #1 and #2 were required in order to get past a first set of errors introduced by the new test. 3. The actual fix -- avoid infinite loop by calling a special new SlotAssignmentWillChange(), rather than ChildrenChanged(), where a minimal GetWithoutInvalidation() is called that does not lead to IsShadowContentRelevantForAccessibility() => FirstChild() => RecalcAssignedNodes() => ChildrenChanged() ... (infinite loop). A simpler potential fix is in CL:2965317 but requires more research. It's also mentioned in a TODO comment. Bug: 1219311 Change-Id: Iafaa289f241a851404ce352715d2970172a2e5f8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2961158 Reviewed-by: Joey Arhar <jarhar@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Commit-Queue: Aaron Leventhal <aleventhal@chromium.org> Cr-Commit-Position: refs/heads/master@{#892778}
mrego
pushed a commit
that referenced
this pull request
Jul 19, 2021
Relative offsets should be applied after fragmentation. Since we perform layout for OOFs once they reach the fragmentation context root (if applicable), we fail to apply any relative offsets at the correct time in the case of inline containing blocks. See CL:2851595 for how this was handled for the non-inline case. The changes required to accomplish this for inline containing blocks include: 1. We currently store an accumulated relative offset in NGContainingBlock inside the OOF node, which is any relative offset from the containing block to the fragmentation context root. We also need to store an accumulated relative offset from the inline container to the containing block in order to properly apply all relative offsets at the time of fragmentation. A new struct, NGInlineContainer, was added to the OOF node to hold the inline container object and the accumulated relative offset to the containing block. 2. A relative offset was also added to the InlineContainingBlockGeometry struct so that we have access to the relative offset from #1 when creating the ContainingBlockInfo for the inline container. 3. The way that relative offsets are applied to inlines, it didn't seem straightforward to separate the relative offset from the normal offset, like we had in CL:2851595. Instead, store the relative offset for the inline and subtract it out from the OOF static position once it reaches the containing block, and subtract it from the containing block rect offset when determining the ContainingBlockInfo for the inline container. 4. Store the total relative offset (from the inline container to the fragmentation context root) in ContainingBlockInfo. This relative offset will then be applied after fragmentation is complete for the OOF as a result of CL:2851595. Bug: 1079031 Change-Id: I7198fec4c01e05ca54c51b2f215569b75b0b869e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2995308 Commit-Queue: Alison Maher <almaher@microsoft.com> Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#902060}
mrego
pushed a commit
that referenced
this pull request
Nov 8, 2021
This patch adds a new produceCropId() API to mediaDevices. This API is called with a DIV or IFRAME element, and adds a new base::UnguessableToken value to that element's rare data structure. This token value will be used in followup patches in order to keep track of an element's location in the page and viewport. Based on the following design document: https://docs.google.com/document/d/1dULARMnMZggfWqa_Ti_GrINRNYXGIli3XK9brzAKEV8/ Bug: 1247761 Change-Id: I01cd67e2d4e3dfa7a86289f876e48c8b55095d0a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3173396 Commit-Queue: Jordan Bayles <jophba@chromium.org> Reviewed-by: Elad Alon <eladalon@chromium.org> Reviewed-by: mark a. foltz <mfoltz@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#925544}
mrego
pushed a commit
that referenced
this pull request
Nov 22, 2021
…eVisibilityKeeper::PrepareToSplitBlockElement()` before splitting a text node It does the following things when caret is collapsed in a text node in a `<p>` or `<div>` element. 1. Split the text node containing caret to insert `<br>` element 2. Insert `<br>` element after it 3. Split ancestor elements which inclusive descendants of the `<p>` or `<div>` 4. Delete the `<br>` element if unnecessary from the left paragraph #3 and #4 are performed by `HTMLEditor::SplitParagraph()` and it calls `WhiteSpaceVisibilityKeeper::PrepareToSplitBlockElement()` correctly before splitting the block. However, in the case (caret is at middle of a text node), the text has already been split to 2 nodes because of #1. Therefore, it fails to handle to keep the white-space visibility. So that I believe that the root cause of this bug is, the method does much complicated things which are required, and doing the redundant things will eat memory space due to undo transactions. However, for now, I'd like to fix this with a simple patch which just call the preparation method before splitting the text node because I'd like to uplift this if it'd be approved (Note that this is not a recent regression, the root cause was created by bug 92686 which was fixed in 17 years ago: <https://searchfox.org/mozilla-central/commit/2e66280faef73e9be218e00758d4eb738395ac83>, but must be annoying bug for users who see this frequently). The new WPTs are pass in Chrome. Differential Revision: https://phabricator.services.mozilla.com/D130950 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1740416 gecko-commit: 73567f6c2bcfa078836a36760498bb11747561dd gecko-reviewers: m_kato, smaug
mrego
pushed a commit
that referenced
this pull request
Feb 8, 2022
By adding new exhaustive tests under ordering/, it was revealed that the ordering between navigatesuccess/navigateerror and the committed/finished promises was not always consistent: 1. Simply adding a currentchange event handler would cause microtasks to run during commit, which changed some ordering. 2. Calling transitionWhile() would take us from the zero-promise case to the 1+-promise case in ScriptPromise::All(). As the new comment explains, both the spec and implementation have an observably-different fast path for the 0-promise case which caused changes in ordering. In the course of fixing this, I found out that the did_finish_before_commit_ code in app_history_api_navigation.{h,cc} was actually not a mitigation for the case it stated, where promises passed to transitionWhile() would settle faster than the browser-process roundtrip for same-document traversals. That is in fact impossible, since we only fire the navigate event after the browser-process roundtrip has completed. Instead, they were a mitigation for (1). This commit then ensures consistent ordering, tested with new rather-exhaustive tests, in the following manner: * We move the firing of currentchange to before resolving the committed promise. This eliminates (1) and allows us to delete the did_finish_before_commit_ tracking. * We always ensure we pass 1+ promises to ScriptPromise::All(). This eliminates (2). A consequence of this is that we are now more likely to get rejected finished promises, in cases like await appHistory.navigate("#1").committed; await appHistory.navigate("#2").committed; Before, the finished promise for the #1 navigation would go through the fast path per (2), and fulfill before the navigation to #2 canceled it. Now that does not happen, so code like the above will give an unhandled promise rejection for #1's finished promise. To avoid this, we unconditionally mark finished promises as handled. This follows some web platform precedent, e.g. stream closed promises, where the promise is one of several information channels (in this case the developer might also find out via the AbortSignal or the navigateerror event). We do *not* do this for the committed promise though, as if a commit fails, that's probably something more deeply wrong, and cannot be ignored. All of these changes will require spec updates. For the tests, we introduce a new ordering/ directory which contains cross-cutting ordering tests, and we consolidate a few tests into the newly-introduced variant-driven exhaustive ones. A couple of other tests were affected by these changes too or fixed as a drive-by. Change-Id: I8a50ca28d009e0a8a2c94331cd17f29b0a8dc463 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3405377 Reviewed-by: Nate Chapin <japhet@chromium.org> Commit-Queue: Domenic Denicola <domenic@chromium.org> Cr-Commit-Position: refs/heads/main@{#963772}
mrego
pushed a commit
that referenced
this pull request
Jun 29, 2022
Now, popups will follow this process when showing/hiding: showPopup(): 1. Move the popup to the top layer, and remove the UA display:none style. 2. Update style. (Transition initial style can be specified in this state.) 3. Set the :top-layer pseudo class. 4. Update style. (Animations/transitions happen here.) hidePopup(): 1. Capture any already-running animations via getAnimations(). 2. Remove the :top-layer pseudo class. 3. Update style. (Animations/transitions start here.) 4. If the hidePopup() call is not due to a "force out" situation, getAnimations() again, remove any from step #1, and then wait here until all of them finish or are cancelled. 4. Remove the popup from the top layer, and add the UA display:none style. 5. Update style. See this issue for more details: openui/open-ui#335 Bug: 1307772 Change-Id: Ia20eb6e9533c1a0b1029ca1279d42fe2648300af Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688871 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1014235}
mrego
pushed a commit
that referenced
this pull request
Jun 29, 2022
See [1] for the origin of this change, which makes it possible to trigger pop up hide animations from within the `hide` event handler. For example: popup.addEventListener('hide', () => { popup.animate({ transform: 'translateY(-50px)', opacity: 0, }, 200); }); To accomplish that, the hide process now looks like this: 1. Capture any already-running animations via getAnimations(), including animations on descendent elements. 2. Remove the :top-layer pseudo class. 3. Fire the 'hide' event. 4. If the hidePopup() call is *not* the result of the pop-up being "forced out" of the top layer, e.g. by a modal dialog or fullscreen element: a. Restore focus to the previously-focused element. b. Update style. (Animations/transitions start here.) c. Call getAnimations() again, remove any from step #1, and then wait until all of them finish or are cancelled. 5. Remove the pop-up from the top layer, and add the UA display:none style. 6. Update style. [1] https://chromium-review.googlesource.com/c/chromium/src/+/3688871/9/third_party/blink/renderer/core/dom/element.cc#2660 Bug: 1307772 Change-Id: I910535b13cfc3c8f8498ed64dae73caa75dd7317 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3708419 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Commit-Queue: Robert Flack <flackr@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1018685}
mrego
pushed a commit
that referenced
this pull request
Jan 18, 2023
****************************************************************** *** SHERIFFS: please don't revert this CL if it causes web_tests to flake/fail. If that happens, the cause is a bad test. Please mark that test as flaky/fail in TestExpectations, with a new crbug. Please block the new bug against crbug.com/1395228. Thanks! ****************************************************************** Prior to this CL, a test like this: ``` <script> window.onload = () => { test((t) => { ... }, 'test 1'); test((t) => { ... }, 'test 2'); test((t) => { ... }, 'test 3'); }; </script> ``` would not run anything after test #1. The issue is that the testharness immediately adds a window load handler that marks `all_loaded = true`, and that ends the tests as soon as the first result from the first test is processed. (The test runner waits for the first test because `Tests.prototype.all_done()` also waits until `this.tests.length > 0`.) There were various mitigating corner cases, such as if you started the list of tests with a promise_test(), that would increment a counter that kept the rest of the tests alive. Etc. With this CL, the testharness-added window.onload handler runs a setTimeout(0), so that `all_loaded` is only set to true after all of the tests are loaded by any window.onload handler. This exposed a few tests that should have been failing but were masked by the lack of test coverage - bugs have been filed for those. Also, several tests that were working around this via various means are also cleaned up in this CL. I'm sure there are more of those. Bug: 1395228,1395226,1307772 Change-Id: I6f12b5922186af4e1e06808ad23b47ceac68559c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4074305 Reviewed-by: Weizhong Xia <weizhong@google.com> Auto-Submit: Mason Freed <masonf@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Mason Freed <masonf@chromium.org> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/main@{#1081558}
mrego
pushed a commit
that referenced
this pull request
Jan 18, 2023
This reverts commit 4a03c6c459fdbf11976a424aa02a1d094484134c. Reason for revert: This has caused tests in upstream WPT to fail, blocking unrelated PRs. It was still possible to upstream this because those tests weren't triggered on the change due to a bug: web-platform-tests#37623 There was an attempted fix for this: web-platform-tests#37549 But, quoting jgraham from the WPT Matrix chat: > the actual fix failed a test I wrote and now I need to spend some more time investigating Original change's description: > WPT: Allow `window.onload` to contain multiple `test()`s > > ****************************************************************** > *** SHERIFFS: please don't revert this CL if it causes web_tests > to flake/fail. If that happens, the cause is a bad > test. Please mark that test as flaky/fail in > TestExpectations, with a new crbug. Please block the > new bug against crbug.com/1395228. Thanks! > ****************************************************************** > > Prior to this CL, a test like this: > > ``` > <script> > window.onload = () => { > test((t) => { ... }, 'test 1'); > test((t) => { ... }, 'test 2'); > test((t) => { ... }, 'test 3'); > }; > </script> > ``` > > would not run anything after test #1. The issue is that the testharness > immediately adds a window load handler that marks `all_loaded = true`, > and that ends the tests as soon as the first result from the first test > is processed. (The test runner waits for the first test because > `Tests.prototype.all_done()` also waits until `this.tests.length > 0`.) > There were various mitigating corner cases, such as if you started > the list of tests with a promise_test(), that would increment a > counter that kept the rest of the tests alive. Etc. > > With this CL, the testharness-added window.onload handler runs a > setTimeout(0), so that `all_loaded` is only set to true after all of > the tests are loaded by any window.onload handler. > > This exposed a few tests that should have been failing but were > masked by the lack of test coverage - bugs have been filed for > those. Also, several tests that were working around this via various > means are also cleaned up in this CL. I'm sure there are more of > those. > > Bug: 1395228,1395226,1307772 > Change-Id: I6f12b5922186af4e1e06808ad23b47ceac68559c > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4074305 > Reviewed-by: Weizhong Xia <weizhong@google.com> > Auto-Submit: Mason Freed <masonf@chromium.org> > Reviewed-by: Mason Freed <masonf@chromium.org> > Commit-Queue: Mason Freed <masonf@chromium.org> > Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1081558} Bug: 1395228,1395226,1307772 Change-Id: Icbddad3a8bb47473bcbc331f424661b9041addf2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4111318 Reviewed-by: David Baron <dbaron@chromium.org> Commit-Queue: Philip Jägenstedt <foolip@chromium.org> Cr-Commit-Position: refs/heads/main@{#1085925}
mrego
pushed a commit
that referenced
this pull request
Jan 18, 2023
In the case that a popover contains an invoker that points back to that invoker, the tab navigation code used to get confused. E.g.: ``` <div id="menu" popover> <button autofocus popoverhidetarget="menu">Button #1</button> <button popoverhidetarget="menu">Button #2</button> </div> ``` In this case, trying to tab between the first and second button would break because the second button appeared to be an invoker for a new popover, when in reality it was an invoker for the same popover. Fixed: 1399601 Bug: 1307772 Change-Id: I276370d7c8eee0dd32f0c89da202a0d3777bf911 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4133482 Commit-Queue: Mason Freed <masonf@chromium.org> Auto-Submit: Mason Freed <masonf@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Reviewed-by: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1089080}
mrego
pushed a commit
that referenced
this pull request
Jun 30, 2023
…each time of the loop There are 2 possible scenarios which are not handled by the method. 1. Moving content node to new `<blockquote>` has already been moved to outside of the editing host. 2. There is no container to insert new `<blockquote>`, e.g., in an inline editing host. In the case #1, we should ignore the ex-child node. In the case #2, we should abort it. Note that Chrome inserts `<blockquote>` even if there is no proper container. However, such behavior is disagreed in interop-2023. Therefore, it's okay just to abort it for now. Depends on D180781 Differential Revision: https://phabricator.services.mozilla.com/D180782 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1756237 gecko-commit: 42f3f3ab11b47f1d56d8bcd6a128398539dd1f23 gecko-reviewers: m_kato
mrego
pushed a commit
that referenced
this pull request
Jun 30, 2023
…eb-platform-tests#40504) * [wdspec] browsingContext.print: fix rounding error in page.py test [pytest](https://github.com/web-platform-tests/wpt/blob/7a087d54be8b6c0ca0181a86dc1ff0b28461c383/webdriver/tests/support/image.py) uses: def cm_to_px(cm): return round(cm * 96 / 2.54) [js](https://github.com/web-platform-tests/wpt/blob/7a087d54be8b6c0ca0181a86dc1ff0b28461c383/tools/wptrunner/wptrunner/print_pdf_runner.html) uses: const viewport = page.getViewport({ scale: 96. / 72. }); ... canvas.height = viewport.height; canvas.width = viewport.width; This produces a rounding error, even though the dimension is correct: > assert cm_to_px(expected_dimensions["height"]) == height E assert 454 == 453 E +454 E -453 The inconsistency of rounding in both ends becomes clear when we eliminate "round" in the pytest side: > assert cm_to_px(expected_dimensions["height"]) == height E assert 453.54330708661416 == 453 E +453.54330708661416 E -453 There are multiple ways to fix this issue. Option #1: Use "floor" instead of "round" in pytest. Option #2: Use a range in the assertion comparison, allowing a difference of up to +-1.0. This is what this PR does. The comparison is performed in [`assert_pdf_dimensions`](https://github.com/web-platform-tests/wpt/blob/b6107cc1ac8b9c2800b4c8e58af719b8e4d9b8db/webdriver/tests/support/fixtures_bidi.py#L210). The problematic part is .96 / .72 which evaluates to 4/3 = 1.333333.... * use floor in cm_to_px instead of round * compare to floor and to round instead of a range * Revert "compare to floor and to round instead of a range" This reverts commit 63f894e. * Revert "use floor in cm_to_px instead of round" This reverts commit 7e65d91.
mrego
pushed a commit
that referenced
this pull request
Apr 26, 2024
…rners We had two issues: 1. Before we had fast rounded corners, we always created mask layers for rounded corner clips, and the mask layer made the scroll begin unreliable and fall back to the main thread. With fast rounded corners, the scrolls were treated as reliable without checking if the point is in or out of the rounded corners. 2. If the scroller has a rounded corner by itself (instead of from an ancestor), as we only create InnerBorderRadiusClip for the contents, the compositor doesn't actually know which part of the layer bounds is transparent to hit test (e.g. if the scroller has a border which is outside of the InnerBorderRadiusClip). Now with HitTestOpaqueness, such layers have HitTestOpaqueness::kMixed. This CL changes the behavior of LayerTreeImpl::FindLayersUpToFirstOpaqueToHitTest (renamed from FindLayerUpToFirstScrollableOrOpaqueToHitTest): - For issue #1: LayerImpl::OpaqueToHitTest() also checks whether the layer is affected by any fast rounded corners; - For issue #2: FindLayerUpToFirstOpaqueToHitTest checks only OpaqueToHitTest() (without checking IsScrollerOrScrollbar()) because a hit test on a scrollable layer is reliable only if it's opaque to hit test. Bug: 40277896 Change-Id: I1acb16f2c6790760661e8239ea1599035f83ea51 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5466909 Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Reviewed-by: Steve Kobes <skobes@chromium.org> Cr-Commit-Position: refs/heads/main@{#1291538}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
dependencies
Pull requests that update a dependency file
github_actions
Pull requests that update Github_actions code
0 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bumps peter-evans/create-pull-request from v2 to v3.8.2.
Release notes
Sourced from peter-evans/create-pull-request's releases.
Commits
052fc72
Merge pull request #724 from peter-evans/fix-assigneesed00d46
fix: use the correct assignees property34371f0
Merge pull request #719 from peter-evans/add-to-listsc27ea51
fix: add to labels and assignees instead of resetting5e9d0ee
Merge pull request #712 from peter-evans/operation-outputb5f41d9
feat: add pull-request-operation output2455e15
Merge pull request #704 from jonico/support-ghes05bc467
Support GitHub Server API URLadc6552
Support GitHub Enterprise Server171fc6c
Merge pull request #701 from peter-evans/update-distributionDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase
.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebase
will rebase this PR@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it@dependabot merge
will merge this PR after your CI passes on it@dependabot squash and merge
will squash and merge this PR after your CI passes on it@dependabot cancel merge
will cancel a previously requested merge and block automerging@dependabot reopen
will reopen this PR if it is closed@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)