Skip to content
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.

Sign up for GitHub

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

🐛 propagate title attribute in amp-iframe #27037

Merged
merged 17 commits into from
Mar 6, 2020
Merged

🐛 propagate title attribute in amp-iframe #27037

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
2541fa3
add title attribute to amp-iframe validator
maxlever Feb 29, 2020
3495d55
add amp-iframe title to bind-validator
maxlever Feb 29, 2020
72248ba
Revert "add title attribute to amp-iframe validator"
maxlever Feb 29, 2020
e5828e7
propagate title as amp-iframe attribute
Mar 2, 2020
00b3f93
only propagate mutated attributes
Mar 2, 2020
7d97094
add test for title propagation
Mar 2, 2020
49da12e
only propagate title
Mar 2, 2020
7c544bf
check if title was mutated before propagating
Mar 2, 2020
7bb7ff5
alphabetize propagated attrs in amp-img
Mar 2, 2020
29b899e
alphabetize propagated attrs in amp-iframe
Mar 2, 2020
82946a4
document why we're only propagating title
Mar 2, 2020
bd178df
add test for mutating 'title' attribute in amp-iframe
Mar 2, 2020
0ee1cfb
actually alphabetize amp-img attrs..
Mar 2, 2020
e1aa813
retrigger travis
Mar 2, 2020
4e75623
retrigger travis pt 2
Mar 3, 2020
a238d7f
retrigger travis pt 3
Mar 3, 2020
de5c997
undo amp-img changes
Mar 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
retrigger travis pt 3
  • Loading branch information
Max Lever committed Mar 3, 2020
commit a238d7f444a77a089c8c34f6364d16b6da50cfa9
2 changes: 1 addition & 1 deletion extensions/amp-iframe/0.1/amp-iframe.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ export class AmpIframe extends AMP.BaseElement {
}
}
if (this.iframe_ && mutations['title']) {
// only propagating title attr because propagating all causes e2e error:
// only propagating title because propagating all causes e2e error:
// See <https://travis-ci.org/ampproject/amphtml/jobs/657440421>
this.propagateAttributes(['title'], this.iframe_);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i would ideally propagate all ATTRIBUTES_TO_PROPAGATE, but this was causing an e2e test error when I tried that, so only propagating title on mutate.

Copy link
Contributor

Choose a reason for hiding this comment

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

A comment here indicating why only title is propagated would be nice.

}
Expand Down