Skip to content

🐛 Adds microsoft-edge protocol#34168

Merged
alanorozco merged 7 commits intoampproject:mainfrom
PHCCorso:main
Jan 6, 2022
Merged

🐛 Adds microsoft-edge protocol#34168
alanorozco merged 7 commits intoampproject:mainfrom
PHCCorso:main

Conversation

@PHCCorso
Copy link
Copy Markdown
Contributor

@PHCCorso PHCCorso commented May 3, 2021

Closes #34167

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 3, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@amp-owners-bot amp-owners-bot bot requested review from alin04 and dreamofabear May 3, 2021 09:33
@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented May 3, 2021 •
edited by ampprojectbot
Loading

Hey @ampproject/wg-caching! These files were changed:

validator/validator-main.protoascii

Gregable
Gregable reviewed May 3, 2021
View reviewed changes
validator/validator-main.protoascii Outdated
protocol: "web+mastodon" # See GitHub issue #14793
protocol: "wh"
protocol: "whatsapp"
protocol: "microsoft-edge"
Copy link
Copy Markdown
Member

@Gregable Gregable May 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment with a link to:
https://docs.microsoft.com/en-us/windows/uwp/launch-resume/launch-default-app#microsoft-edge-uri-scheme

or something similar.

Copy link
Copy Markdown
Contributor Author

@PHCCorso PHCCorso May 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gregable Sure.

Added it 👍 .

Copy link
Copy Markdown
Member

@alanorozco alanorozco May 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks the validation entry needs to be sorted alphabetically as well.

Copy link
Copy Markdown
Contributor Author

@PHCCorso PHCCorso May 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alanorozco done.

@Gregable Gregable requested review from alanorozco and removed request for alin04 May 4, 2021 16:34
@honeybadgerdontcare
Copy link
Copy Markdown
Contributor

honeybadgerdontcare commented May 10, 2021

@alanorozco could you triage or review, Will is no longer on AMP and unsure who owns amp-bind

alanorozco
alanorozco reviewed May 10, 2021
View reviewed changes
extensions/amp-bind/0.1/bind-validator.js Outdated
'web+mastodon': true,
'wh': true,
'whatsapp': true,
'microsoft-edge': true,
Copy link
Copy Markdown
Member

@alanorozco alanorozco May 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep list alphabetically sorted below // 3rd Party Protocols.

microsoft-edge should go between line and skype.

Copy link
Copy Markdown
Contributor Author

@PHCCorso PHCCorso May 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alanorozco Yup, fixed. 👍

Copy link
Copy Markdown
Member

@alanorozco alanorozco May 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to sort it on this file as well.

Copy link
Copy Markdown
Contributor Author

@PHCCorso PHCCorso May 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alanorozco silly me. Totally overlooked that. Fixed.

Copy link
Copy Markdown
Contributor Author

@PHCCorso PHCCorso Jul 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alanorozco ?

@alanorozco
Copy link
Copy Markdown
Member

alanorozco commented May 10, 2021

Looks good to me except for sorting. I'll approve once that's addressed.

@alanorozco
Copy link
Copy Markdown
Member

alanorozco commented May 17, 2021

@phcorso You need to sort it on bind-validator.js as well.

@alanorozco
Copy link
Copy Markdown
Member

alanorozco commented Oct 7, 2021 •
edited
Loading

@PHCCorso Apologies for the late response. Would love to merge this once able. Would you mind updating your branch?
https://app.circleci.com/pipelines/github/ampproject/amphtml/9428/workflows/f8e6c0f1-be51-49c3-92cc-4853b42f405c/jobs/144978

ERROR: pull/34168 is missing the latest CircleCI config revision 3592c8c0676f5e73e6b8ab1248a57119cff85d93.


This can be fixed in three ways:


1. Click the "Update branch" button on GitHub and follow instructions.
   ⤷ It can be found towards the bottom of the PR, after the list of checks.


2. Pull the latest commits from main and re-push the PR branch.
   ⤷ Follow these steps:

      git checkout main
      git pull
      git checkout <PR branch>
      git merge main
      git push origin


3. Rebase on main and re-push the PR branch.
   ⤷ Follow these steps:

      git checkout main
      git pull
      git checkout <PR branch>
      git rebase main
      git push origin --force



Exited with code exit status 1

@PHCCorso
Copy link
Copy Markdown
Contributor Author

PHCCorso commented Oct 25, 2021

@alanorozco done.

@alanorozco alanorozco merged commit da7957f into ampproject:main Jan 6, 2022
westonruter added a commit that referenced this pull request Jan 10, 2022
@westonruter
Merge branch 'main' of github.com:ampproject/amphtml into add/importa…
df9a488 
…nce-attr-to-hero-img

* 'main' of github.com:ampproject/amphtml: (525 commits)
  mathml storybook: supply missing component definition. (#37326)
  storybook: Iframe --> BentoIframe (#37327)
  🖍  [Story system layer] New ad badge (#37311)
  🐛 [amp story] Replay/next page button bug fix (#37316)
  🚀  [Story performance] Remove affiliate links (#37280)
  Compiler: Add amp-carousel-0.1 to the builder map (#37308)
  ⏪  [Story system layer] Audio icon disappears when story has background audio (#37314)
  🚀  [Story performance] Remove story access (#37281)
  Fix remapping esbuild output on Windows (#37312)
  🐛 adds in correct weight for amp-story-product-tag text (#37188)
  typechecking carousel: remove shame files (#37213)
  Use remapping to remap minified sourcemap into source code (#37238)
  SwG Release 0.1.22.199 (#37310)
  🐛 Adds microsoft-edge protocol (#34168)
  Sync for validator cpp engine and cpp htmlparser (#37292)
  ✨ amp-story-shopping Updated currency with product price and correct Localized currency (#37249)
  ✨[Smartadserver ad extension] Implement support for Fast Fetch (#36991)
  Remove client-side-experiments-config.json from this repo (#37304)
  🚮  Remove closure compiler logic from build-system. (#37296)
  🌐 Added RTL ordering i18n for amp story shopping tag (#37252)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Gregable Gregable Gregable approved these changes

@alanorozco alanorozco alanorozco approved these changes

@dreamofabear dreamofabear Awaiting requested review from dreamofabear

Assignees

No one assigned

Labels

PR use: In Beta / Experimental PR use: In Stable WG: caching

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Microsoft Edge redirection mistakenly considered as HTTP protocol

7 participants

@PHCCorso @CLAassistant @honeybadgerdontcare @alanorozco @Gregable @twifkak @ampprojectbot