Skip to content

Commit f78eb35

Browse files
honeybadgerdontcaretwifkak
authored andcommitted
When heights or sizes is empty, the layout should be fixed not responsive.
The AMP Runtime interprets an img with a height and width plus an empty heights or sizes as layout=fixed. Currently the transformers and validator treat it as layout=responsive. PiperOrigin-RevId: 279189628
1 parent e5d7817 commit f78eb35

File tree

4 files changed

+33
-7
lines changed

4 files changed

+33
-7
lines changed

transformer/internal/htmlnode/htmlnode.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,18 @@ func HasAttribute(n *html.Node, namespace, key string) bool {
131131
return ok
132132
}
133133

134+
// HasAttributeAndIsNotEmpty return true if the node has the attribute named
135+
// with 'key' and it's value is not empty.
136+
func HasAttributeAndIsNotEmpty(n *html.Node, namespace, key string) bool {
137+
if v, ok := GetAttributeVal(n, namespace, key); ok {
138+
if v != "" {
139+
return true
140+
}
141+
return false
142+
}
143+
return false
144+
}
145+
134146
// SetAttribute overrides the value of the attribute on node n with
135147
// namespace and key with val. If the attribute doesn't exist, it adds it.
136148
func SetAttribute(n *html.Node, namespace, key, val string) {

transformer/layout/layout.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ func ApplyLayout(n *html.Node) error {
8686
}
8787
actualLayout, err := getNormalizedLayout(
8888
inputLayout, dimensions,
89-
htmlnode.GetAttributeValOrNil(n, "", "sizes"),
90-
htmlnode.GetAttributeValOrNil(n, "", "heights"))
89+
htmlnode.HasAttributeAndIsNotEmpty(n, "", "sizes"),
90+
htmlnode.HasAttributeAndIsNotEmpty(n, "", "heights"))
9191
if err != nil {
9292
return err
9393
}
@@ -143,7 +143,7 @@ func getNormalizedDimensions(n *html.Node, layout amppb.AmpLayout_Layout) (cssDi
143143

144144
// getNormalizedLayout returns the normalized AmpLayout based on the
145145
// provided dimensions, or an error if it is not supported.
146-
func getNormalizedLayout(layout amppb.AmpLayout_Layout, dimensions cssDimensions, sizes, heights *string) (amppb.AmpLayout_Layout, error) {
146+
func getNormalizedLayout(layout amppb.AmpLayout_Layout, dimensions cssDimensions, sizes, heights bool) (amppb.AmpLayout_Layout, error) {
147147
var result amppb.AmpLayout_Layout
148148
if layout != amppb.AmpLayout_UNKNOWN {
149149
result = layout
@@ -153,7 +153,7 @@ func getNormalizedLayout(layout amppb.AmpLayout_Layout, dimensions cssDimensions
153153
result = amppb.AmpLayout_FLUID
154154
} else if dimensions.height.isSet && (!dimensions.width.isSet || dimensions.width.isAuto) {
155155
result = amppb.AmpLayout_FIXED_HEIGHT
156-
} else if dimensions.height.isSet && dimensions.width.isSet && (sizes != nil || heights != nil) {
156+
} else if dimensions.height.isSet && dimensions.width.isSet && (sizes || heights) {
157157
result = amppb.AmpLayout_RESPONSIVE
158158
} else {
159159
result = amppb.AmpLayout_FIXED

transformer/layout/layout_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,20 @@ func TestApplyLayout(t *testing.T) {
114114
html.Attribute{Key: "height", Val: "100"}, html.Attribute{Key: "layout", Val: "fixed-height"}),
115115
`<amp-img height="100" layout="fixed-height" class="i-amphtml-layout-fixed-height i-amphtml-layout-size-defined" style="height:100px;" i-amphtml-layout="fixed-height"></amp-img>`,
116116
},
117+
{
118+
"Fixed when heights is empty",
119+
htmlnode.Element(
120+
"amp-img",
121+
html.Attribute{Key: "height", Val: "100"}, html.Attribute{Key: "heights", Val: ""}, html.Attribute{Key: "width", Val: "300"}),
122+
`<amp-img height="100" heights="" width="300" class="i-amphtml-layout-fixed i-amphtml-layout-size-defined" style="width:300px;height:100px;" i-amphtml-layout="fixed"></amp-img>`,
123+
},
124+
{
125+
"Fixed when sizes is empty",
126+
htmlnode.Element(
127+
"amp-img",
128+
html.Attribute{Key: "height", Val: "100"}, html.Attribute{Key: "sizes", Val: ""}, html.Attribute{Key: "width", Val: "300"}),
129+
`<amp-img height="100" sizes="" width="300" class="i-amphtml-layout-fixed i-amphtml-layout-size-defined" style="width:300px;height:100px;" i-amphtml-layout="fixed"></amp-img>`,
130+
},
117131
{
118132
"Responsive",
119133
htmlnode.Element(

transformer/transformers/serversiderendering.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,13 +133,13 @@ func canRemoveBoilerplate(n *html.Node) bool {
133133
if n.Data == amphtml.AMPAudio {
134134
return false
135135
}
136-
if htmlnode.HasAttribute(n, "", "heights") {
136+
if htmlnode.HasAttributeAndIsNotEmpty(n, "", "heights") {
137137
return false
138138
}
139-
if htmlnode.HasAttribute(n, "", "media") {
139+
if htmlnode.HasAttributeAndIsNotEmpty(n, "", "media") {
140140
return false
141141
}
142-
if htmlnode.HasAttribute(n, "", "sizes") {
142+
if htmlnode.HasAttributeAndIsNotEmpty(n, "", "sizes") {
143143
return false
144144
}
145145
}

0 commit comments

Comments
 (0)