Skip to content

Commit

Permalink
Add support for link imagesrcset and imagesizes
Browse files Browse the repository at this point in the history
Adds validator support for the two new `<link rel=preload>` attributes, and adds srcset URL rewriting for imagesrcset.

Fixes ampproject/amphtml#29302

PiperOrigin-RevId: 325359140
  • Loading branch information
Googler authored and twifkak committed Aug 13, 2020
1 parent da8a000 commit bc7ac09
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 25 deletions.
35 changes: 20 additions & 15 deletions transformer/transformers/absoluteurl.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ var /* const */ imgTagAttrs = []string{"longdesc"}
// * action-xhr
// * Any <img> tag with attribute:
// * longdesc
// * Any <link> tag with attribute:
// * imagesrcset
//
// URLs in stylesheets and srcsets are handled by the ExternalUrlRewrite
// transformer.
Expand Down Expand Up @@ -146,8 +148,13 @@ func AbsoluteURL(e *Context) error {
amphtml.ToAbsoluteURL(documentURL, e.BaseURL, href.Val))
}
}
if _, ok := htmlnode.FindAttribute(n, "", "srcset"); ok {
rewriteSrcsetURLs(n, documentURL, e.BaseURL)
if srcset, ok := htmlnode.FindAttribute(n, "", "srcset"); ok {
htmlnode.SetAttribute(n, "", "srcset", rewriteSrcsetURLs(documentURL, e.BaseURL, srcset.Val))
}
if n.DataAtom == atom.Link {
if srcset, ok := htmlnode.FindAttribute(n, "", "imagesrcset"); ok {
htmlnode.SetAttribute(n, "", "imagesrcset", rewriteSrcsetURLs(documentURL, e.BaseURL, srcset.Val))
}
}
}
return nil
Expand All @@ -165,18 +172,16 @@ func rewriteAbsoluteURLs(n *html.Node, documentURL string, baseURL *url.URL,
}
}

func rewriteSrcsetURLs(n *html.Node, documentURL string, baseURL *url.URL) {
if v, ok := htmlnode.GetAttributeVal(n, "", "srcset"); ok {
normalized, offsets := amphtml.ParseSrcset(v)
var sb strings.Builder
var pos int
for _, element := range offsets {
sb.WriteString(normalized[pos:element.Start])
sb.WriteString(amphtml.ToAbsoluteURL(
documentURL, baseURL, normalized[element.Start:element.End]))
pos = element.End
}
sb.WriteString(normalized[pos:])
htmlnode.SetAttribute(n, "", "srcset", sb.String())
func rewriteSrcsetURLs(documentURL string, baseURL *url.URL, srcset string) string {
normalized, offsets := amphtml.ParseSrcset(srcset)
var sb strings.Builder
var pos int
for _, element := range offsets {
sb.WriteString(normalized[pos:element.Start])
sb.WriteString(amphtml.ToAbsoluteURL(
documentURL, baseURL, normalized[element.Start:element.End]))
pos = element.End
}
sb.WriteString(normalized[pos:])
return sb.String()
}
8 changes: 8 additions & 0 deletions transformer/transformers/absoluteurl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,14 @@ func TestAbsoluteURLTansformer(t *testing.T) {
baseURL: "http://www.example.com",
documentURL: fooURL,
},
{
desc: "imagesrcset rewritten",
input: "<link imagesrcset=\"200.png 200w, 400.png 400w\">",
expected: "<link imagesrcset=\"https://example.com/200.png 200w, " +
"https://example.com/400.png 400w\">",
baseURL: "https://example.com/",
documentURL: barURL,
},
}
for _, tc := range tcs {
rawInput := tt.Concat(
Expand Down
39 changes: 29 additions & 10 deletions transformer/transformers/urlrewrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ type nodeMap map[string]*html.Node
// * <amp-img/amp-anim/img srcset>
// * <image href> / <image xlink:href> which are SVG-specific images.
// * <link rel=icon href>
// * <link rel=preload as=image href>
// * <link rel=preload imagesrcset>
// * <amp-video poster>
// * <use xlink:href>
// * a background image given in the <style amp-custom> tag / style attribute
Expand Down Expand Up @@ -104,12 +106,20 @@ func URLRewrite(e *Context) error {

switch n.Data {
case "link":
// Rewrite 'href' attribute within <link rel="icon" href=...> and variants
// to point into the AMP Cache.
if htmlnode.HasAttribute(n, "", "href") {
if v, ok := htmlnode.GetAttributeVal(n, "", "rel"); ok && fieldsContain(v, "icon") {
if rel, hasRel := htmlnode.GetAttributeVal(n, "", "rel"); hasRel {
// Rewrite 'href' attribute within <link rel="icon" href=...> and variants
// to point into the AMP Cache.
if fieldsContain(rel, "icon") {
ctx.parseSimpleImageAttr(n, "", "href")
}
// Rewrite the 'href' and 'imagesrcset' attributes within <link rel="preload" as="image">
// to point into the AMP Cache.
if fieldsContain(rel, "preload") {
if as, ok := htmlnode.GetAttributeVal(n, "", "as"); ok && as == "image" {
ctx.parseSimpleImageAttr(n, "", "href")
}
}
ctx.parseExistingSrcset(n, "", "imagesrcset")
}

case "amp-img", "amp-anim", "img":
Expand All @@ -120,7 +130,7 @@ func URLRewrite(e *Context) error {
}

if v, srcsetOk := htmlnode.GetAttributeVal(n, "", "srcset"); srcsetOk && len(v) > 0 {
ctx.parseExistingSrcset(n, v)
ctx.parseExistingSrcset(n, "", "srcset")
} else if srcOk {
ctx.parseNewSrcset(n, src)
}
Expand Down Expand Up @@ -346,11 +356,20 @@ func writeAndMark(sb *strings.Builder, pos *int, s string) {
}

// parseExistingSrcset normalizes and parses the srcset attribute.
func (ctx *urlRewriteContext) parseExistingSrcset(n *html.Node, srcset string) {
normalized, offsets := amphtml.ParseSrcset(srcset)
nc := elementNodeContext{n, "", "srcset", offsets}
*ctx = append(*ctx, &nc)
htmlnode.SetAttribute(n, "", "srcset", normalized)
func (ctx *urlRewriteContext) parseExistingSrcset(n *html.Node, namespace, attrName string) bool {
srcset, ok := htmlnode.FindAttribute(n, namespace, attrName)
if ok {
normalized, offsets := amphtml.ParseSrcset(srcset.Val)
if len(offsets) == 0 {
htmlnode.RemoveAttribute(n, srcset)
return false
}
htmlnode.SetAttribute(n, namespace, attrName, normalized)
nc := elementNodeContext{n, namespace, attrName, offsets}
*ctx = append(*ctx, &nc)
return true
}
return false
}

// Do not add srcset for responsive layout if the width attribute is smaller
Expand Down
36 changes: 36 additions & 0 deletions transformer/transformers/urlrewrite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,42 @@ func TestURLRewrite_link(t *testing.T) {
expected: `<link rel="icon shortcut" href="https://www-example-com.cdn.ampproject.org/i/www.example.com/foo"/>`,
base: "http://www.example.com",
},
{
desc: `link rel="preload" as="image" href`,
input: `<link rel="preload" as="image" href=foo>`,
expected: `<link rel="preload" as="image" href="https://www-example-com.cdn.ampproject.org/i/www.example.com/foo"/>`,
base: "http://www.example.com",
},
{
desc: `link rel="preload" as="image" imagesrcset`,
input: `<link rel="preload" as="image" imagesrcset="/foo.jpg, bar.jpg 50w">`,
expected: `<link rel="preload" as="image" imagesrcset="https://www-example-com.cdn.ampproject.org/i/www.example.com/foo.jpg 1x, https://www-example-com.cdn.ampproject.org/i/www.example.com/bar.jpg 50w"/>`,
base: "http://www.example.com",
},
{
desc: `link rel="preload" as="image" href and imagesrcset`,
input: `<link rel="preload" as="image" href=foo.jpg imagesrcset="/bar.jpg, baz.jpg 50w">`,
expected: `<link rel="preload" as="image" href="https://www-example-com.cdn.ampproject.org/i/www.example.com/foo.jpg" imagesrcset="https://www-example-com.cdn.ampproject.org/i/www.example.com/bar.jpg 1x, https://www-example-com.cdn.ampproject.org/i/www.example.com/baz.jpg 50w"/>`,
base: "http://www.example.com",
},
{
desc: `link rel="preload" as="image" badly formed imagesrcset removed`,
input: `<link rel="preload" as="image" imagesrcset="/bar.jpg baz.jpg 50w">`,
expected: `<link rel="preload" as="image"/>`,
base: "http://www.example.com",
},
{
desc: `link rel="preload" href requires as="image" `,
input: `<link rel="preload" href="/bar.jpg">`,
expected: `<link rel="preload" href="/bar.jpg"/>`,
base: "http://www.example.com",
},
{
desc: `link rel="preload" href does not require as="image" `,
input: `<link rel="preload" imagesrcset="/foo.jpg, bar.jpg 50w">`,
expected: `<link rel="preload" imagesrcset="https://www-example-com.cdn.ampproject.org/i/www.example.com/foo.jpg 1x, https://www-example-com.cdn.ampproject.org/i/www.example.com/bar.jpg 50w"/>`,
base: "http://www.example.com",
},
}
runURLRewriteTestcases(t, tcs)
}
Expand Down

0 comments on commit bc7ac09

Please sign in to comment.