[WebShareTarget] Update manifest to conform with spec. |
||||||
Issue descriptionWebShareTarget spec: https://wicg.github.io/web-share-target/ The spec has been updated to replace |url_template| with |action| and |params|. Previously |url_template| had a for similar to https://example.com/share?title={title} where {title} would be replaced with the share data. The new spec replaces this with |action| = https://example.com/share and |params.title| = title. Note this allows query parameter keys with custom names so if |params.title| = foo, the equivalent |url_template| in the old format would be https://example.com/share?foo={title}. A first target for this API is WebAPKs so the bulk of the effort will be invested in getting that working. Work Phases: 1. Add |action| and |params| to all code relating the the WebAppManifest. 2. Replace all usages of |url_template| with |action| and |params| (hopefully over a few CLs). 3. Update WebShare and WebAPKs to use |action| and |params|. 4. Delete |url_template| from the manifest. If time permits: 5. Add support for |method| (GET or POST (GET by default)) and |enctype|.
,
Aug 7
For context; this CL contains a rough version of most of the changes required. https://chromium-review.googlesource.com/c/chromium/src/+/1157264 I don't plan to submit it and ideally I will break it up. But that is what the final state will roughly look like % any reviewer comments or errors found.
,
Aug 8
Thanks for doing this! (Just want to make sure: is pkotwicz@ across this? Since he is responsible for the WebAPK implementation of WST. Want to make sure there's no doubling of effort.) FYI you can manually test this here: https://wicg.github.io/web-share-target/demos/sharetarget.html I've updated that test app to respond to both the old and new APIs, and if you're still using the old API, it prints "Your browser is using the deprecated 'url_template' Web Share Target API." So once updated, that site should still work properly and not print that message. I'm also going to look into implementing some web platform tests for the new API.
,
Aug 8
Yep, ckitagawa@ is interning with our team and is working with pkotwicz@ and hanxi@ on this.
,
Aug 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c334b1b812f41f6140f94cdc584b12dbb74c1d53 commit c334b1b812f41f6140f94cdc584b12dbb74c1d53 Author: Calder Kitagawa <ckitagawa@chromium.org> Date: Mon Aug 13 23:47:17 2018 [WebShareTarget] Update Manifest to add action and params Phase 1 of the work to replace url_template with action and params as per the updated WebShareTarget spec: https://wicg.github.io/web-share-target/. Note: the work to add method and enctype will be done at a later date. This is primarily an effort to deprecate url_template and replace it with action and params. To avoid a large CL this work is being done in stages with this CL just adding the new fields the the manifest without deleting the old one to avoid breakages. See the bug for a detailed plan for performing these changes. Bug: 871788 Change-Id: I68e574ab88a6d9bf48c9adc9c8a13600aafd1761 Reviewed-on: https://chromium-review.googlesource.com/1165483 Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Matt Giuca <mgiuca@chromium.org> Reviewed-by: Xi Han <hanxi@chromium.org> Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org> Cr-Commit-Position: refs/heads/master@{#582749} [modify] https://crrev.com/c334b1b812f41f6140f94cdc584b12dbb74c1d53/third_party/blink/common/manifest/manifest.cc [modify] https://crrev.com/c334b1b812f41f6140f94cdc584b12dbb74c1d53/third_party/blink/common/manifest/manifest_mojom_traits.cc [modify] https://crrev.com/c334b1b812f41f6140f94cdc584b12dbb74c1d53/third_party/blink/public/common/manifest/manifest.h [modify] https://crrev.com/c334b1b812f41f6140f94cdc584b12dbb74c1d53/third_party/blink/public/common/manifest/manifest_mojom_traits.h [modify] https://crrev.com/c334b1b812f41f6140f94cdc584b12dbb74c1d53/third_party/blink/public/mojom/manifest/manifest.mojom
,
Aug 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9ecf1e9febd0228ff314bde970fb1e4bf383771c commit 9ecf1e9febd0228ff314bde970fb1e4bf383771c Author: Calder Kitagawa <ckitagawa@chromium.org> Date: Fri Aug 17 19:16:52 2018 [WebShareTarget] Update manifest parser to use action and params Phase 2 of the work to replace url_template with action and params as per the updated WebShareTarget spec: https://wicg.github.io/web-share-target/. This work stops the parser from parsing |url_template| for |share_target| and instead parses |action| and |params|. This will effectively disable any downstream dependencies which trigger when |url_template| is populated. Disabled codepaths are in chrome/browser/webshare and chrome/android/webapk. This will be quickly remedied before M70. See the bug for a detailed plan for performing these changes. Bug: 871788 Change-Id: If15f1158ae439286c7a2d64538842c21fd1b9d5f Reviewed-on: https://chromium-review.googlesource.com/1168115 Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Xi Han <hanxi@chromium.org> Cr-Commit-Position: refs/heads/master@{#584146} [modify] https://crrev.com/9ecf1e9febd0228ff314bde970fb1e4bf383771c/content/renderer/manifest/manifest_parser.cc [modify] https://crrev.com/9ecf1e9febd0228ff314bde970fb1e4bf383771c/content/renderer/manifest/manifest_parser.h [modify] https://crrev.com/9ecf1e9febd0228ff314bde970fb1e4bf383771c/content/renderer/manifest/manifest_parser_unittest.cc
,
Aug 21
,
Aug 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ee19ff939ef30bb4f1c7dcbb4431f4be47bec29d commit ee19ff939ef30bb4f1c7dcbb4431f4be47bec29d Author: Calder Kitagawa <ckitagawa@chromium.org> Date: Wed Aug 22 17:54:28 2018 [WebShareTarget] Update to action/params in WebShare Phase 3 of the work to replace url_template with action and params as per the updated WebShareTarget spec: https://wicg.github.io/web-share-target/. This work updates WebShare to use the new action and parameter model rather than |url_template|. See the bug for a detailed plan for performing these changes. Bug: 871788 Change-Id: I0d2e044320085a082d4a8912de2a099fe01f9156 Reviewed-on: https://chromium-review.googlesource.com/1168127 Reviewed-by: Xi Han <hanxi@chromium.org> Reviewed-by: Matt Giuca <mgiuca@chromium.org> Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org> Cr-Commit-Position: refs/heads/master@{#585152} [modify] https://crrev.com/ee19ff939ef30bb4f1c7dcbb4431f4be47bec29d/chrome/browser/ui/views/webshare/webshare_target_picker_view_unittest.cc [modify] https://crrev.com/ee19ff939ef30bb4f1c7dcbb4431f4be47bec29d/chrome/browser/webshare/share_service_impl.cc [modify] https://crrev.com/ee19ff939ef30bb4f1c7dcbb4431f4be47bec29d/chrome/browser/webshare/share_service_impl_unittest.cc [modify] https://crrev.com/ee19ff939ef30bb4f1c7dcbb4431f4be47bec29d/chrome/browser/webshare/share_target_pref_helper.cc [modify] https://crrev.com/ee19ff939ef30bb4f1c7dcbb4431f4be47bec29d/chrome/browser/webshare/share_target_pref_helper_unittest.cc [modify] https://crrev.com/ee19ff939ef30bb4f1c7dcbb4431f4be47bec29d/chrome/browser/webshare/webshare_target.cc [modify] https://crrev.com/ee19ff939ef30bb4f1c7dcbb4431f4be47bec29d/chrome/browser/webshare/webshare_target.h
,
Aug 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18 commit d112f7cb0ac8965bb6ad8404e7bebfeb78869b18 Author: Calder Kitagawa <ckitagawa@chromium.org> Date: Thu Aug 23 16:07:02 2018 [WebShareTarget] WebAPKs add action/param support Phase 3 of the work to replace url_template with action and params as per the updated WebShareTarget spec: https://wicg.github.io/web-share-target/. This work updates WebAPKs to use the new action and parameter model rather than |url_template|. See the bug for a detailed plan for performing these changes. Bug: 871788 Change-Id: I17ea1f7cb0fa5b6bef0e1ad1cf90434f0bacb60f Reviewed-on: https://chromium-review.googlesource.com/1171587 Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org> Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org> Reviewed-by: Dominick Ng <dominickn@chromium.org> Reviewed-by: Matt Giuca <mgiuca@chromium.org> Reviewed-by: Xi Han <hanxi@chromium.org> Cr-Commit-Position: refs/heads/master@{#585503} [modify] https://crrev.com/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18/chrome/android/webapk/libs/common/src/org/chromium/webapk/lib/common/WebApkMetaDataKeys.java [modify] https://crrev.com/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18/chrome/android/webapk/shell_apk/AndroidManifest.xml [modify] https://crrev.com/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18/chrome/android/webapk/shell_apk/BUILD.gn [modify] https://crrev.com/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18/chrome/android/webapk/shell_apk/bound_manifest_config.json [modify] https://crrev.com/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18/chrome/android/webapk/shell_apk/http_manifest_config.json [add] https://crrev.com/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18/chrome/android/webapk/shell_apk/junit/src/org/chromium/webapk/shell_apk/ShareActivityTest.java [modify] https://crrev.com/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18/chrome/android/webapk/shell_apk/shell_apk_version.gni [modify] https://crrev.com/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18/chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/ShareActivity.java [modify] https://crrev.com/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18/chrome/browser/android/shortcut_info.cc [modify] https://crrev.com/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18/chrome/browser/android/shortcut_info.h [modify] https://crrev.com/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18/chrome/browser/android/webapk/webapk.proto [modify] https://crrev.com/d112f7cb0ac8965bb6ad8404e7bebfeb78869b18/chrome/browser/android/webapk/webapk_installer.cc
,
Aug 23
For WebAPKs the bulk of the client-side changes are in and the server-side changes are done. I'm hoping to land: https://chromium-review.googlesource.com/c/chromium/src/+/1185847 Before the end of my internship, which deletes url_template and resolves up to and including phase 4 of the work scoped by this bug. Phase 5 is left to whomever takes over this work. The following are know issues/TODOs: 1) Update the WebAPK update request triggering logic (WebApkUpdateManager::needsUpdate()). The following CL contains most of this work, other than tests, the challenge here is figuring out how to extract the meta-data keys from the ShareActivity and comparing it to the data coming from the parsed manifest. - https://chromium-review.googlesource.com/c/chromium/src/+/1185522 2) The URL encoding logic in - chrome/browser/webshare/share_service_impl.cc - chrome/android/webapk/shell_apk/src/org/chromium/webapk/shell_apk/ShareActivity.java is not entirely correct according the present spec as Uri.Builder and net::EscapeQueryParamValue don't follow https://url.spec.whatwg.org/#application/x-www-form-urlencoded but rather use a "normal" encoding (that of encodeURIComponent). However, there is an ongoing discussion on the encoding that needs to be resolved to determine which encoding to use. See https://github.com/WICG/web-share-target/issues/59 3) Phase 5 of the work to add POST and enctype support as originally described in comment 1 still needs to be done. (See the spec for details https://wicg.github.io/web-share-target/) Reassigning to hanxi@.
,
Aug 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c48ae8bf3e2ed5fb8e0834399071ef4db4b99736 commit c48ae8bf3e2ed5fb8e0834399071ef4db4b99736 Author: Calder Kitagawa <ckitagawa@chromium.org> Date: Sat Aug 25 09:39:16 2018 [WebShareTarget] Remove url_template This is phase 4 of the work to update the WebShareTarget spec. This CL removes the old style url_template. All clients only use the new action and parameter based model. Bug: 871788 Change-Id: I0c4b3a20a4afa0d1b24bde7f52941d399ac7048e Reviewed-on: https://chromium-review.googlesource.com/1185847 Reviewed-by: Xi Han <hanxi@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Matt Giuca <mgiuca@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#586132} [modify] https://crrev.com/c48ae8bf3e2ed5fb8e0834399071ef4db4b99736/content/renderer/manifest/manifest_parser.cc [modify] https://crrev.com/c48ae8bf3e2ed5fb8e0834399071ef4db4b99736/content/renderer/manifest/manifest_parser.h [modify] https://crrev.com/c48ae8bf3e2ed5fb8e0834399071ef4db4b99736/content/renderer/manifest/manifest_parser_unittest.cc [modify] https://crrev.com/c48ae8bf3e2ed5fb8e0834399071ef4db4b99736/third_party/blink/common/BUILD.gn [modify] https://crrev.com/c48ae8bf3e2ed5fb8e0834399071ef4db4b99736/third_party/blink/common/manifest/manifest_mojom_traits.cc [delete] https://crrev.com/72acdf58284a507f427c1a54c36a23f10646679f/third_party/blink/common/manifest/manifest_share_target_util.cc [delete] https://crrev.com/72acdf58284a507f427c1a54c36a23f10646679f/third_party/blink/common/manifest/manifest_share_target_util_unittest.cc [modify] https://crrev.com/c48ae8bf3e2ed5fb8e0834399071ef4db4b99736/third_party/blink/public/common/BUILD.gn [modify] https://crrev.com/c48ae8bf3e2ed5fb8e0834399071ef4db4b99736/third_party/blink/public/common/manifest/manifest.h [modify] https://crrev.com/c48ae8bf3e2ed5fb8e0834399071ef4db4b99736/third_party/blink/public/common/manifest/manifest_mojom_traits.h [delete] https://crrev.com/72acdf58284a507f427c1a54c36a23f10646679f/third_party/blink/public/common/manifest/manifest_share_target_util.h [modify] https://crrev.com/c48ae8bf3e2ed5fb8e0834399071ef4db4b99736/third_party/blink/public/mojom/manifest/manifest.mojom
,
Aug 30
,
Oct 9
CHanges are live on canary
,
Oct 10
Works for me. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by hanxi@chromium.org
, Aug 7Components: Mobile>WebAPKs