Replace LESS Customizer with CSS Custom Variables

benlk commented 8 years ago

Time tracking here: HELPDESK-389

A few sites we're hosting on WP Engine are being marked as not mobile-friendly (https://www.google.com/webmasters/tools/mobile-friendly/) . One possible cause appears to be that Google is not recognizing the generated css file created by Largo's Custom CSS option as a valid stylesheet.

In NPQ's case:

homepage fails the test: test results
the generated CSS is interpreted as the homepage, suggesting that the URL parameter gets stripped: test results
the child theme's CSS file is interpreted as not-html on the CDN: test results
the child theme's CSS file is interpreted as not-html not on the CDN: test results

So it looks like Google might not be seeing the responsive styles in the generated stylesheet because it strips URL parameters.

Proposed course of testing:

[ ] make sure that Google stripping URL params is what's happening, with a quick-and-dirty test page written somewhere: echo url params and some test content

If the above is correct:

[x] test whether largo detects stylesheets at a non-.css URL

If it does:

[ ] rename the URL for generated stylesheets
[ ] preserve URL params used on the generated stylesheet
[ ] base url (example: /css) should return the appropriate stylesheet for Largo: minified/not, customized output

aschweigert commented 8 years ago

It's worth noting that not ALL sites using the less>css customizer exhibit this behavior so there may be other issues at play. This is something we should investigate and try to get to the bottom of though just to rule this out as a potential culprit in the NPQ issue.

benlk commented 8 years ago

http://f.benlk.com/inn/npq/noname-test-2/ : stylesheet with content-type: text/html is not interpreted as a stylesheet

http://f.benlk.com/inn/npq/noname-test-3/ : stylesheet with content-type: text/css is interpreted as a stylesheet

Conclusion: Google does not need file extensions to determine stylesheet-ness

benlk commented 8 years ago

http://f.benlk.com/inn/npq/urlparams/?cheese=yes : results say that google does not strip arbitrary URL parameters.

But does it strip the params used?

benlk commented 8 years ago

https://www.google.com/webmasters/tools/mobile-friendly/?url=http%3A%2F%2Ff.benlk.com%2Finn%2Fnpq%2Furlparams%2F%3Fver%3D0.5.4%26largo_custom_less_variable%3D1%26css_file%3Dstyle.min.css%26timestamp%3D2016-01-15%252018%3A20%3A36%26cheese%3Dyes : Google does not strip the parameters. :disappointed:

Compare the test results with http://f.benlk.com/inn/npq/urlparams/?ver=0.5.4&largo_custom_less_variable=1&css_file=style.min.css&timestamp=2016-01-15%2018:20:36&cheese=yes

All parameters are preserved.

benlk commented 8 years ago

If you use the useragent string used by the Google page checker:

Mozilla/5.0 (iPhone, CPU iPhone OS 8_3 like Mac OS X) AppleWebKit/600.1.4 (KHTML, like Gecko) Version/8.0 Mobile/12F70 Safari/600.1.4 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)

attempting to reach http://gijn.org/?ver=0.5.4&largo_custom_less_variable=1&css_file=style.min.css&timestamp=2016-02-12%2018:11:47 will redirect you to http://gijn.org/, and trying to request the stylesheet will fail because of that redirect.

This apparently only happens with the Googlebot user-agent, though.

wget --verbose -U "Mozilla/5.0 (iPhone, CPU iPhone OS 8_3 like Mac OS X) AppleWebKit/600.1.4 (KHTML, like Gecko) Version/8.0 Mobile/12F70 Safari/600.1.4 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)" http://gijn.org/?ver=0.5.4&largo_custom_less_variable=1&css_file=style.min.css&timestamp=2016-02-12%2018:11:47 
[1] 117491
[2] 117492
[3] 117493
# --2016-02-18 10:22:07--  http://gijn.org/?ver=0.5.4
Resolving gijn.org... 45.79.65.60
Connecting to gijn.org|45.79.65.60|:80... connected.
HTTP request sent, awaiting response... 301 Moved Permanently
Location: http://gijn.org/ [following]
--2016-02-18 10:22:07--  http://gijn.org/
Reusing existing connection to gijn.org:80.
HTTP request sent, awaiting response... 200 OK
Length: unspecified [text/html]
Saving to: `index.html?ver=0.5.4'

    [ <=>                                                                                ] 66,534      --.-K/s   in 0.04s   

2016-02-18 10:22:07 (1.56 MB/s) - `index.html?ver=0.5.4' saved

So the next question: Why is Googlebot getting a 301 redirect away from the stylesheet?

aschweigert commented 8 years ago

well that's really strange

benlk commented 8 years ago

From the NPQ logs:

66.249.79.119 nonprofitquarterly.org - [18/Feb/2016:07:10:34 +0000] "GET /?ver=0.5.4&largo_custom_less_variable=1&css_file=style.min.css&timestamp=2016-01-15%2018:20:36 HTTP/1.0" 301 178 "https://nonprofitquarterly.org/2016/02/17/student-debt-more-onerous-depending-on-gender-and-race/" "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)"

6.249.79.119 nonprofitquarterly.org - [18/Feb/2016:07:21:14 +0000] "GET /?ver=0.5.4&largo_custom_less_variable=1&css_file=style.min.css&timestamp=2016-01-15%2018:20:36 HTTP/1.0" 301 178 "-" "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)"

301 indicates it's a redirect, and 178 is apparently the size of the response in bytes: https://stackoverflow.com/questions/9234699/understanding-apache-access-log

Doesn't really help, but confirms that it's a 301 redirect.

aschweigert commented 8 years ago

removing the redirect resolved the issue. let's go back and check the other sites that were failing just to make sure but i think this is ok to close

benlk commented 8 years ago

Sites still failing appear to be because of robots.txt issues. Compare http://nonprofitquarterly.org/robots.txt and http://citylimits.org/robots.txt, which formerly had this problem, with the following:

ctmirror.org because of robots.txt. http://ctmirror.org/robots.txt has a bunch of stuff that other sites don't have. I'm looking at the *? and *?*
http://georgianewslab.org/robots.txt same deal
http://inewsnetwork.org/robots.txt and
http://insideenergy.org/robots.txt
http://iowawatch.org/robots.txt

I stopped here, but can finish checking all the failing sites if you want.

aschweigert commented 8 years ago

Right, we talked about this. That's because Largo 0.3 adds some rules to robots.txt that prevents bots from accessing those files. I'm pro-removing those (even though that means patching 0.3 which is not something we would ordinarily do).

benlk commented 8 years ago

Derp, right. Will that be its own issue, or should the PR for that fix in 0.3 just reference this fix?

aschweigert commented 8 years ago

just fix it as part of this issue so we can close this out.

benlk commented 8 years ago

Fixed in the v0.3-maintenance branch

benlk commented 6 years ago

This is also caused by https://wpengine.com/support/redirecting-bots-how-this-benefits-you/

Because the Largo URL /?ver=0.5.5.2&largo_custom_less_variable=1&css_file=style.min.css&timestamp=0 is entirely composed of query args, WPE redirects it to /.

This can be fixed by asking WPE to disable the redirector.

benlk commented 6 years ago

Questions:

[ ] can we change the URL provided for the CSS generator to get the stylesheet at a URL that does not experience this problem?
- [ ] prefixing the query args with a word to make them load properly: /foo?ver=0.5.5.2&largo_custom_less_variable=1&css_file=style.min.css&timestamp=0 — would need to be tested on a Largo site that had the redirector enabled.
- [ ] building a page that will output the stylesheet at a fixed URL, without the query args being necessary

benlk commented 5 years ago

Replacing this feature with CSS variables would also solve the problem, by getting rid of this strange stylesheet URL. Throw a stack of CSS variables in the header, modify the custom properties in response to theme options for colors, but the loaded stylesheets remain the same.

benlk commented 5 years ago

For the 0.7 research level:

[ ] are CSS Custom Properties at the point where we'd feel comfortable using them in production?
[ ] what shims exist for fallback support for browsers that don't support CSS Custom Properties?
[ ] what does the CSS Custom Properties look like at the specification level? Is there an upcoming refresh anticipated?

Then:

[ ] do we want to replace the serverside LESS compiler with some CSS Custom Properties output as described in https://github.com/INN/largo/issues/1137#issuecomment-467064972 ?

benlk commented 5 years ago

Supported by everyone except Internet Explorer (all versions): https://caniuse.com/#feat=css-variables

This is currently a "Candidate Recommendation", but people have been using it in production since 2017, and Smash Magazine recommended it in April 2017: https://www.smashingmagazine.com/2017/04/start-using-css-custom-properties/

The CSSWG mailing list and GitHub issues have been pretty quiet on the subject of CSS variables in the last 18 months; I don't think there's an update planned.

Shims:

linked to by CanIUse, has no dependencies, MIT license: https://github.com/nuxodin/ie11CustomProperties
no shim available in the Modernizr library that we use to shim some other stuff.

IE11 is still receiving security updates on Windows 7, 8.1, Server 2012, and Embedded 8. https://en.wikipedia.org/wiki/Internet_Explorer_11 — do we want to drop feature parity for IE 11?

IE is ~5% of US traffic, according to https://gs.statcounter.com/browser-market-share/desktop-mobile-tablet/united-states-of-america/#monthly-201807-201907-bar

MirandaEcho commented 5 years ago

Replace LESS Customizer with CSS Custom Variables

benlk commented 5 years ago

Resolved in the meeting today:

yes on customizer
yes on IE
take 2h to gut and replace, then update the estimate

benlk commented 4 years ago

add a separate color field for the "donate" button color, because people want to set that separately it seems

WPBuddy / largo

Replace LESS Customizer with CSS Custom Variables #1137