Remove video-id from the amp-youtube extension runtime.#4507
Merged
powdercloud merged 1 commit intoampproject:masterfrom Aug 30, 2016
powdercloud:video-id-no-more
Merged
Remove video-id from the amp-youtube extension runtime.#4507powdercloud merged 1 commit intoampproject:masterfrom powdercloud:video-id-no-more
powdercloud merged 1 commit intoampproject:masterfrom
powdercloud:video-id-no-more
Conversation
been deprecated for a while and has already been removed from the validator. #4367
Contributor
|
LGTM since this goes hand in hand with the validator change, but do we know how many pages are expected to get invalidated as result of the validator removing videoid? (Just wondering if we should keep the backward compatibility or it is a good time to remove it) |
Contributor
Author
|
@aghassemi The validator already enforces that video-id can't be used, so this can't break any valid AMPHTML document. |
jridgewell
pushed a commit
that referenced
this pull request
Sep 15, 2016
* Implement anchor href expand on document click with X/Y support * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * Update amp-ad-network-doubleclick-impl.md * Reverting doc changes The previous change was made in error (wrong branch). * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix pull conflicts * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix test failure due to issue after merge * address PR comments * Implement anchor href expand on document click with X/Y support * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix pull conflicts * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * changes in response to PR review * fix test failure due to issue after merge * Revert "reenable history tests (#4758)" This reverts commit 1375883. * Revert "Reverting doc changes" This reverts commit 8d8c6dd. * Revert "Small Validator Updates, including blacklisting __amp_source_origin for URLs. (#4781)" This reverts commit bffeb2d. * Revert "Remove video-id from the amp-youtube extension runtime. This attr had (#4507)" This reverts commit 8d1799d. * Revert unintentionally added files. * Revert unintentionally added file. * Fix test failure from latest merge * Fix conflict and lint errors * Fix lint errors, type check failures, and test failures
dreamofabear
pushed a commit
to dreamofabear/amphtml
that referenced
this pull request
Sep 16, 2016
…roject#4773) * Implement anchor href expand on document click with X/Y support * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * Update amp-ad-network-doubleclick-impl.md * Reverting doc changes The previous change was made in error (wrong branch). * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix pull conflicts * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix test failure due to issue after merge * address PR comments * Implement anchor href expand on document click with X/Y support * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix pull conflicts * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * changes in response to PR review * fix test failure due to issue after merge * Revert "reenable history tests (ampproject#4758)" This reverts commit 1375883. * Revert "Reverting doc changes" This reverts commit 8d8c6dd. * Revert "Small Validator Updates, including blacklisting __amp_source_origin for URLs. (ampproject#4781)" This reverts commit bffeb2d. * Revert "Remove video-id from the amp-youtube extension runtime. This attr had (ampproject#4507)" This reverts commit 8d1799d. * Revert unintentionally added files. * Revert unintentionally added file. * Fix test failure from latest merge * Fix conflict and lint errors * Fix lint errors, type check failures, and test failures
jridgewell
pushed a commit
that referenced
this pull request
Sep 20, 2016
… support (#5053) * Implement anchor href expand on document click with X/Y support * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * Update amp-ad-network-doubleclick-impl.md * Reverting doc changes The previous change was made in error (wrong branch). * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix pull conflicts * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix test failure due to issue after merge * address PR comments * Implement anchor href expand on document click with X/Y support * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix pull conflicts * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * changes in response to PR review * fix test failure due to issue after merge * Revert "reenable history tests (#4758)" This reverts commit 1375883. * Revert "Reverting doc changes" This reverts commit 8d8c6dd. * Revert "Small Validator Updates, including blacklisting __amp_source_origin for URLs. (#4781)" This reverts commit bffeb2d. * Revert "Remove video-id from the amp-youtube extension runtime. This attr had (#4507)" This reverts commit 8d1799d. * Revert unintentionally added files. * Revert unintentionally added file. * Fix test failure from latest merge * Fix conflict and lint errors * Fix lint errors, type check failures, and test failures * Fix test failures * Remove Math.random override causing breakages in other tests * PR comment feedback * PR review changes * PR code review * PR review * Address error in issue #5113 * PR review
mityaha
pushed a commit
to brightcove-archive/ooyala_amphtml
that referenced
this pull request
Nov 30, 2016
…ampproject#4507) been deprecated for a while and has already been removed from the validator. ampproject#4367
mityaha
pushed a commit
to brightcove-archive/ooyala_amphtml
that referenced
this pull request
Nov 30, 2016
…roject#4773) * Implement anchor href expand on document click with X/Y support * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * Update amp-ad-network-doubleclick-impl.md * Reverting doc changes The previous change was made in error (wrong branch). * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix pull conflicts * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix test failure due to issue after merge * address PR comments * Implement anchor href expand on document click with X/Y support * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix pull conflicts * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * changes in response to PR review * fix test failure due to issue after merge * Revert "reenable history tests (ampproject#4758)" This reverts commit 1375883. * Revert "Reverting doc changes" This reverts commit 8d8c6dd. * Revert "Small Validator Updates, including blacklisting __amp_source_origin for URLs. (ampproject#4781)" This reverts commit bffeb2d. * Revert "Remove video-id from the amp-youtube extension runtime. This attr had (ampproject#4507)" This reverts commit 8d1799d. * Revert unintentionally added files. * Revert unintentionally added file. * Fix test failure from latest merge * Fix conflict and lint errors * Fix lint errors, type check failures, and test failures
mityaha
pushed a commit
to brightcove-archive/ooyala_amphtml
that referenced
this pull request
Nov 30, 2016
… support (ampproject#5053) * Implement anchor href expand on document click with X/Y support * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * Update amp-ad-network-doubleclick-impl.md * Reverting doc changes The previous change was made in error (wrong branch). * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix pull conflicts * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix test failure due to issue after merge * address PR comments * Implement anchor href expand on document click with X/Y support * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * fix javadoc error * changes in response to PR review * update var substititions documentation * make capture listener actually use capture! * remove unnecessary else clause * fix pull conflicts * Fixes after full stack testing: need to look at event path due to event target rewrite for shadowDom elements; was not binding whether capture to click handler properly. Test coverage for fixes to be added shortly * update tests; instead of looking for shadowRoot boundary to update x/y offset, look for existence of event.path and use event.target which should match host element * slight refactor for readability * changes in response to PR review * fix test failure due to issue after merge * Revert "reenable history tests (ampproject#4758)" This reverts commit 1375883. * Revert "Reverting doc changes" This reverts commit 8d8c6dd. * Revert "Small Validator Updates, including blacklisting __amp_source_origin for URLs. (ampproject#4781)" This reverts commit bffeb2d. * Revert "Remove video-id from the amp-youtube extension runtime. This attr had (ampproject#4507)" This reverts commit 8d1799d. * Revert unintentionally added files. * Revert unintentionally added file. * Fix test failure from latest merge * Fix conflict and lint errors * Fix lint errors, type check failures, and test failures * Fix test failures * Remove Math.random override causing breakages in other tests * PR comment feedback * PR review changes * PR code review * PR review * Address error in issue ampproject#5113 * PR review
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
This attr had been deprecated for a while and has already been removed from the validator.
Some discussion about amp-youtube attrs happeneing in #4367