Skip to content

Add URL rewriting and srcset support to amp-bind#7206

Merged
dreamofabear merged 6 commits intoampproject:masterfrom
dreamofabear:bind-url-rewrite-and-srcset
Jan 27, 2017
Merged

Add URL rewriting and srcset support to amp-bind#7206
dreamofabear merged 6 commits intoampproject:masterfrom
dreamofabear:bind-url-rewrite-and-srcset

Conversation

@dreamofabear
Copy link
Copy Markdown

Partial for #6199.

  • Rewrite image URLs on CDN-served AMP pages
  • Support srcset attribute in BindValidator

@dreamofabear
Copy link
Copy Markdown
Author

/to @aghassemi /cc @dvoytenko

Copy link
Copy Markdown
Contributor

@aghassemi aghassemi left a comment

Choose a reason for hiding this comment

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

Look good, just a small change request.

blockedURLs: ['__amp_source_origin'],
},
// TODO: Support `srcset`.
srcset: {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

there is no srcset on amp-video

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Woops, done.

'AMP-VIDEO', 'src', '__amp_source_origin')).to.be.false;

// srcset
expect(val.isResultValid(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done.

Copy link
Copy Markdown
Author

@dreamofabear dreamofabear left a comment

Choose a reason for hiding this comment

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

Thanks for the review.

blockedURLs: ['__amp_source_origin'],
},
// TODO: Support `srcset`.
srcset: {
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Woops, done.

'AMP-VIDEO', 'src', '__amp_source_origin')).to.be.false;

// srcset
expect(val.isResultValid(
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done.

@dreamofabear dreamofabear force-pushed the bind-url-rewrite-and-srcset branch from 8837022 to aaf60de Compare January 27, 2017 19:31
@dreamofabear dreamofabear merged commit 4ecfb21 into ampproject:master Jan 27, 2017
@dreamofabear dreamofabear deleted the bind-url-rewrite-and-srcset branch January 27, 2017 20:05
jridgewell pushed a commit to jridgewell/amphtml that referenced this pull request Jan 31, 2017
* add srcset support

* fix types

* rewrite urls for cache

* ignore compiler warnings in third_party/caja

* whitelist amp-bind to depend on sanitizer.js

* ali's comments
torch2424 pushed a commit to torch2424/amphtml that referenced this pull request Feb 14, 2017
* add srcset support

* fix types

* rewrite urls for cache

* ignore compiler warnings in third_party/caja

* whitelist amp-bind to depend on sanitizer.js

* ali's comments
mrjoro pushed a commit to mrjoro/amphtml that referenced this pull request Apr 28, 2017
* add srcset support

* fix types

* rewrite urls for cache

* ignore compiler warnings in third_party/caja

* whitelist amp-bind to depend on sanitizer.js

* ali's comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants