-
Notifications
You must be signed in to change notification settings - Fork 72
Proposal for sidePanel lifecycle events. #779
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
base: main
Are you sure you want to change the base?
Conversation
Partial draft
Added the remaining sections.
Updated schema: - added PanelInfo - Removed reasons Removed un-asked use cases Elaborated on the workarounds Added points to the open discussions
Updated workarounds
Updated reasons in open discussions.
Fixed typos
Moved content from future work to Open discussions.
Related issue: #693 |
Gentle reminder for reviews on this @Rob--W, @oliverdunk, @xeenon. |
} | ||
} | ||
``` | ||
- Would adding onHidden, onShown or onVisibilityChanged events for side panel extensions make sense too? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in the last meeting, I am waiting until there is more discussion in the proposal about this before doing a full review.
Hey! I have two questions about the current proposal:
The spec says this event is fired when a side panel is closed. But it’s not clear who will receive this event. Can you confirm if this event is only sent to the extension that originally opened the side panel? It would be helpful if this behavior could be made clear in the spec.
Is it necessary to include the path property in the event payload? Since the extension itself sets the path when it opens the side panel, it can maintain the context internally and can track this information on its own. Also, there isn’t a standard pattern in other extension APIs where |
Add the chrome.sidePanel.onOpened event. This event is triggered whenever the side panel is opened, in all scenarios. Returns tabId (only for tab-specific panels), windowId and path. Reference: w3c/webextensions#779 Bug: 434215968 Change-Id: I2911b653f1078c770a7c1f140ba79cd9a0e82124 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6656092 Reviewed-by: Devlin Cronin <[email protected]> Reviewed-by: Oliver Dunk <[email protected]> Commit-Queue: Oliver Dunk <[email protected]> Cr-Commit-Position: refs/heads/main@{#1502167}
Add the chrome.sidePanel.onClosed event. This event is triggered whenever the side panel is closed / replaced, in all scenarios. Returns tabId (only for tab-specific panels), windowId and path. Reference: w3c/webextensions#779 Bug: 387966419 Change-Id: I09c78cbb216aaad879fd2126c8fc9d1fc824cc42 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6651789 Reviewed-by: Tom Lukaszewicz <[email protected]> Reviewed-by: Justin Lulejian <[email protected]> Commit-Queue: Harsh Singh <[email protected]> Cr-Commit-Position: refs/heads/main@{#1504006}
This reverts commit 4be076c. Reason for revert: Failures on bot Example failure: https://ci.chromium.org/ui/p/chromium/builders/ci/Win10%20Tests%20x64/102940/test-results?q=ID%3Aninja%3A%2F%2Fchrome%2Ftest%3Ainteractive_ui_tests%2FAIOnDeviceBrowserTest.ExposedToWindowOrWorker%2FFlagNotEnabledByUser_WithWorkerFlag_NoAPIKillswitch_NoOTToken+V%3Abuilder%3DWin10+Tests+x64+V%3Aos%3DWindows-10-19045+V%3Atest_suite%3Dinteractive_ui_tests Test history for all failures: https://ci.chromium.org/ui/test/chromium/ninja%3A%2F%2Fchrome%2Ftest%3Ainteractive_ui_tests%2FAIOnDeviceBrowserTest.ExposedToWindowOrWorker%2FFlagNotEnabledByUser_WithWorkerFlag_NoAPIKillswitch_NoOTToken?q=VHASH%3A03afe4e28bf770c7 Bug: 387966419 Original change's description: > [Extensions] Implement sidePanel.onClosed > > Add the chrome.sidePanel.onClosed event. This event is triggered > whenever the side panel is closed / replaced, in all scenarios. Returns > tabId (only for tab-specific panels), windowId and path. > > Reference: w3c/webextensions#779 > > Bug: 387966419 > Change-Id: I09c78cbb216aaad879fd2126c8fc9d1fc824cc42 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6651789 > Reviewed-by: Tom Lukaszewicz <[email protected]> > Reviewed-by: Justin Lulejian <[email protected]> > Commit-Queue: Harsh Singh <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1504006} Bug: 387966419 No-Presubmit: true No-Tree-Checks: true No-Try: true Change-Id: Iba08de39e29b5100441a3487496c3752ad7c3794 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6867434 Bot-Commit: Rubber Stamper <[email protected]> Commit-Queue: Salma Elmahallawy <[email protected]> Owners-Override: Salma Elmahallawy <[email protected]> Cr-Commit-Position: refs/heads/main@{#1504136}
This reverts commit 3ae6786. Reason for revert: tests failing on bots see history: https://ci.chromium.org/ui/test/chromium/ninja%3A%2F%2Fchrome%2Ftest%3Abrowser_tests%2FExtensionOpenSidePanelBrowserTest.OpenSidePanel_OpenContextualPanelInActiveTab?q=VHASH%3A15e8c90d55ca90bb Bug: 434215968 Original change's description: > [Extensions] Implement sidePanel.onOpened > > Add the chrome.sidePanel.onOpened event. This event is triggered > whenever the side panel is opened, in all scenarios. Returns tabId > (only for tab-specific panels), windowId and path. > > Reference: w3c/webextensions#779 > > Bug: 434215968 > Change-Id: I2911b653f1078c770a7c1f140ba79cd9a0e82124 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6656092 > Reviewed-by: Devlin Cronin <[email protected]> > Reviewed-by: Oliver Dunk <[email protected]> > Commit-Queue: Oliver Dunk <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1502167} Bug: 434215968 Change-Id: Iadaf47329dfaa0fc57a189936d77e400aff88fcb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6867414 Owners-Override: Salma Elmahallawy <[email protected]> Bot-Commit: Rubber Stamper <[email protected]> Reviewed-by: Justin Lulejian <[email protected]> Reviewed-by: Solomon Kinard <[email protected]> Commit-Queue: Salma Elmahallawy <[email protected]> Cr-Commit-Position: refs/heads/main@{#1504171}
This is a reland of commit 3ae6786 The original commit was reverted in https://chromium-review.googlesource.com/c/chromium/src/+/6867414 due to a failing browser test: ExtensionOpenSidePanelBrowserTest.OpenSidePanel_OpenContextualPanelInActiveTab After discussion, it was determined that this change was not the root cause of the test failure, which is now believed to have been a flake. This commit relands the original CL to proceed with the feature implementation. Original change's description: > [Extensions] Implement sidePanel.onOpened > > Add the chrome.sidePanel.onOpened event. This event is triggered > whenever the side panel is opened, in all scenarios. Returns tabId > (only for tab-specific panels), windowId and path. > > Reference: w3c/webextensions#779 > > Bug: 434215968 > Change-Id: I2911b653f1078c770a7c1f140ba79cd9a0e82124 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6656092 > Reviewed-by: Devlin Cronin <[email protected]> > Reviewed-by: Oliver Dunk <[email protected]> > Commit-Queue: Oliver Dunk <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1502167} Bug: 434215968 Change-Id: I52ab7eea1bacf290207b220432873e416a073bcd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6881067 Reviewed-by: Tom Lukaszewicz <[email protected]> Commit-Queue: Harsh Singh <[email protected]> Reviewed-by: Justin Lulejian <[email protected]> Reviewed-by: Solomon Kinard <[email protected]> Reviewed-by: Devlin Cronin <[email protected]> Cr-Commit-Position: refs/heads/main@{#1506083}
This is a reland of commit 4be076c The original commit was reverted in https://chromium-review.googlesource.com/c/chromium/src/+/6867434 due to a failing interactive UI test: AIOnDeviceBrowserTest.ExposedToWindowOrWorker/FlagNotEnabledByUser_WithWorkerFlag_NoAPIKillswitch_NoOTToken After discussion, it was determined that this change was not the root cause of the test failure, which is now believed to have been a flake. This commit relands the original CL to proceed with the feature implementation. Original change's description: > [Extensions] Implement sidePanel.onClosed > > Add the chrome.sidePanel.onClosed event. This event is triggered > whenever the side panel is closed / replaced, in all scenarios. Returns > tabId (only for tab-specific panels), windowId and path. > > Reference: w3c/webextensions#779 > > Bug: 387966419 > Change-Id: I09c78cbb216aaad879fd2126c8fc9d1fc824cc42 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6651789 > Reviewed-by: Tom Lukaszewicz <[email protected]> > Reviewed-by: Justin Lulejian <[email protected]> > Commit-Queue: Harsh Singh <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1504006} Bug: 387966419 Change-Id: I8f0eb4935d9eda17b25c728594b8b5ad8dc8b93f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6881965 Reviewed-by: Solomon Kinard <[email protected]> Commit-Queue: Oliver Dunk <[email protected]> Reviewed-by: Devlin Cronin <[email protected]> Reviewed-by: Justin Lulejian <[email protected]> Reviewed-by: Oliver Dunk <[email protected]> Reviewed-by: Tom Lukaszewicz <[email protected]> Cr-Commit-Position: refs/heads/main@{#1506553}
This reverts commit 40a9499. Reason for revert: Two builder failures right after merge 1. [Linux ChromiumOS MSan Tests] https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/54600/overview 2. [Linux UBSan Tests] https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20UBSan%20Tests/9580/overview Bug: 387966419 Original change's description: > Reland "[Extensions] Implement sidePanel.onClosed" > > This is a reland of commit 4be076c > > The original commit was reverted in > https://chromium-review.googlesource.com/c/chromium/src/+/6867434 > due to a failing interactive UI test: > AIOnDeviceBrowserTest.ExposedToWindowOrWorker/FlagNotEnabledByUser_WithWorkerFlag_NoAPIKillswitch_NoOTToken > > After discussion, it was determined that this change was not the root > cause of the test failure, which is now believed to have been a flake. > This commit relands the original CL to proceed with the feature > implementation. > > Original change's description: > > [Extensions] Implement sidePanel.onClosed > > > > Add the chrome.sidePanel.onClosed event. This event is triggered > > whenever the side panel is closed / replaced, in all scenarios. Returns > > tabId (only for tab-specific panels), windowId and path. > > > > Reference: w3c/webextensions#779 > > > > Bug: 387966419 > > Change-Id: I09c78cbb216aaad879fd2126c8fc9d1fc824cc42 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6651789 > > Reviewed-by: Tom Lukaszewicz <[email protected]> > > Reviewed-by: Justin Lulejian <[email protected]> > > Commit-Queue: Harsh Singh <[email protected]> > > Cr-Commit-Position: refs/heads/main@{#1504006} > > Bug: 387966419 > Change-Id: I8f0eb4935d9eda17b25c728594b8b5ad8dc8b93f > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6881965 > Reviewed-by: Solomon Kinard <[email protected]> > Commit-Queue: Oliver Dunk <[email protected]> > Reviewed-by: Devlin Cronin <[email protected]> > Reviewed-by: Justin Lulejian <[email protected]> > Reviewed-by: Oliver Dunk <[email protected]> > Reviewed-by: Tom Lukaszewicz <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1506553} Bug: 387966419 No-Presubmit: true No-Tree-Checks: true No-Try: true Change-Id: Id6cbaee618b7cd7a920907fe863934b26ef0df4e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6884054 Owners-Override: Zonghan Xu <[email protected]> Bot-Commit: Rubber Stamper <[email protected]> Reviewed-by: Solomon Kinard <[email protected]> Commit-Queue: Zonghan Xu <[email protected]> Cr-Commit-Position: refs/heads/main@{#1506664}
Expose onOpened event in documentation. Reference: w3c/webextensions#779 Bug: 434215968 Change-Id: I157dae60c77e136fa45159ad717a7ee4eeaa01a7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6893955 Reviewed-by: Devlin Cronin <[email protected]> Commit-Queue: Oliver Dunk <[email protected]> Reviewed-by: Oliver Dunk <[email protected]> Cr-Commit-Position: refs/heads/main@{#1508523}
Add the chrome.sidePanel.onClosed event. This event is triggered whenever the side panel is closed / replaced, in all scenarios. Returns tabId (only for tab-specific panels), windowId and path. Reference: w3c/webextensions#779 Bug: 387966419 Change-Id: I356f1a3446bfa44b1948b1ce6e41c7f3fcd19102 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6903718 Reviewed-by: Kelvin Jiang <[email protected]> Reviewed-by: Tom Lukaszewicz <[email protected]> Commit-Queue: Harsh Singh <[email protected]> Reviewed-by: Devlin Cronin <[email protected]> Cr-Commit-Position: refs/heads/main@{#1515854}
This reverts commit b780699. Reason for revert: Suspecting this to have caused the compile error (tree closure): https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8703593717140416609/+/u/compile/raw_io.output_text_failure_summary_ Original change's description: > [Extensions] Implement sidePanel.onClosed > > Add the chrome.sidePanel.onClosed event. This event is triggered > whenever the side panel is closed / replaced, in all scenarios. Returns > tabId (only for tab-specific panels), windowId and path. > > Reference: w3c/webextensions#779 > > Bug: 387966419 > Change-Id: I356f1a3446bfa44b1948b1ce6e41c7f3fcd19102 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6903718 > Reviewed-by: Kelvin Jiang <[email protected]> > Reviewed-by: Tom Lukaszewicz <[email protected]> > Commit-Queue: Harsh Singh <[email protected]> > Reviewed-by: Devlin Cronin <[email protected]> > Cr-Commit-Position: refs/heads/main@{#1515854} Bug: 387966419 No-Presubmit: true No-Tree-Checks: true No-Try: true Change-Id: Ic2e1fab262b04222ff1865f3daf85754b729dc82 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6950594 Commit-Queue: Leon Masopust <[email protected]> Bot-Commit: Rubber Stamper <[email protected]> Owners-Override: Leon Masopust <[email protected]> Cr-Commit-Position: refs/heads/main@{#1515921}
Creates a proposal for the extensions sidePanel API events which will cater to basic lifecycle events like onClosed and onOpened.