[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