Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: include.preload.js

Issue 29770568: Issue 6645 - Collapse elements through user style sheets if possible (Closed)
Patch Set: Created May 4, 2018, 2:17 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: include.preload.js
===================================================================
--- a/include.preload.js
+++ b/include.preload.js
@@ -131,41 +131,43 @@
return urls;
}
-function isCollapsibleMediaElement(element, mediatype)
+function getSelectorForUserStyleSheet(element)
{
- if (mediatype != "MEDIA")
- return false;
-
- if (!element.getAttribute("src"))
- return false;
+ // Setting the "display" CSS property to "none" doesn't have any effect on
+ // <frame> elements (in framesets). So we have to hide it inline through
+ // the "visisiblity" CSS property.
+ if (element.localName == "frame")
+ return null;
- for (let child of element.children)
+ // If the <video> or <audio> element contains any <source> or <track>
+ // children, we cannot address it in CSS by the source URL; in that case we
+ // don't "collapse" it using a CSS selector but rather hide it directly by
+ // setting the style="..." attribute.
+ if (element.localName == "video" || element.localName == "audio")
{
- // If the <video> or <audio> element contains any <source> or <track>
- // children, we cannot address it in CSS by the source URL; in that case we
- // don't "collapse" it using a CSS selector but rather hide it directly by
- // setting the style="..." attribute.
- if (child.localName == "source" || child.localName == "track")
- return false;
+ for (let child of element.children)
+ {
+ if (child.localName == "source" || child.localName == "track")
+ return null;
+ }
}
- return true;
-}
-
-function collapseMediaElement(element, srcValue)
-{
- if (!srcValue)
- return;
+ let selector = element.localName;
+ let hasSrc = false;
+ for (let attr of ["src", "srcset"])
+ {
+ if (attr in element)
+ {
+ let value = element.getAttribute(attr);
+ if (value)
+ {
+ selector += "[" + attr + "=" + CSS.escape(value) + "]";
+ hasSrc = true;
+ }
+ }
+ }
- let selector = element.localName + "[src=" + CSS.escape(srcValue) + "]";
-
- // Adding selectors is expensive so do it only if we really have a new
- // selector.
- if (!collapsingSelectors.has(selector))
- {
- collapsingSelectors.add(selector);
- elemhide.addSelectors([selector], null, "collapsing", true);
- }
+ return hasSrc ? selector : null;
}
function hideElement(element)
@@ -205,11 +207,9 @@
if (urls.length == 0)
return;
- let collapsibleMediaElement = isCollapsibleMediaElement(element, mediatype);
-
- // Save the value of the src attribute because it can change between now and
- // when we get the response from the background page.
- let srcValue = collapsibleMediaElement ? element.getAttribute("src") : null;
+ // Construct the selector here, because the attributes it relies on can change
+ // between now and when we get the response from the background page.
+ let selector = getSelectorForUserStyleSheet(element);
browser.runtime.sendMessage(
{
@@ -222,10 +222,18 @@
{
if (collapse)
{
- if (collapsibleMediaElement)
- collapseMediaElement(element, srcValue);
+ if (selector)
+ {
+ if (!collapsingSelectors.has(selector))
+ {
+ collapsingSelectors.add(selector);
+ elemhide.addSelectors([selector], null, "collapsing", true);
+ }
+ }
else
+ {
hideElement(element);
+ }
}
}
);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld