[wp-meta] [Making WordPress.org] #3752: Sponsor logos in the Sponsors widget get removed by ad blockers

Making WordPress.org noreply at wordpress.org
Tue Mar 12 23:49:29 UTC 2019


#3752: Sponsor logos in the Sponsors widget get removed by ad blockers
--------------------------------------+---------------------
 Reporter:  coreymckrill              |       Owner:  (none)
     Type:  defect                    |      Status:  new
 Priority:  low                       |   Milestone:
Component:  WordCamp Site & Plugins   |  Resolution:
 Keywords:  good-first-bug has-patch  |
--------------------------------------+---------------------

Comment (by iandunn):

 Thanks! The patch looks pretty good at first glance. A few possible
 improvements:

 * We probably also want to target the `[sponsors]` shortcode, which would
 probably be `.wcorg-sponsor-description img`.
 * `widget.innerHTML = '<p class="message">' + sponsorWidget.message +
 '</p>' ;` looks like it could be an XSS vector if the string weren't
 hardcoded; it might be better to [create a new DOM node
 programatically](https://vip.wordpress.com/documentation/vip-go/vip-code-
 review/javascript-security-best-practices/#escaping-dynamic-javascript-
 values), just to make it obvious that it's safe, and avoid the change of a
 vulnerability being introduced in the future if the string is updated to
 include user input.
 * We might want to make the `querySelectorAll` lookup only target children
 of `. wcb_widget_sponsors` to avoid false positives elsewhere on the page.
 * I think it'd be nicer to create a normal CSS rule in the `.css` file,
 and just add the corresponding class to the new DOM element, rather than
 setting the CSS from JS.
 * `a` isn't a meaningful variable name, something like `logo` would be
 more descriptive/obvious
 * `<p class="message">`: something more specific might be helpful to avoid
 conflicts, like `sponsored-blocked`. There may even be a conventional name
 that AdBlock, etc use for this purpose, to ensure that it won't also be
 blocked.

 There's a few minor things that running `phpcs` will catch as well.

 xref: https://wordpress.slack.com/archives/C08M59V3P/p1546728991052200

-- 
Ticket URL: <https://meta.trac.wordpress.org/ticket/3752#comment:6>
Making WordPress.org <https://meta.trac.wordpress.org/>
Making WordPress.org


More information about the wp-meta mailing list