Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client-side dynamic adding of "is-active" class is not occuring when site is on Pantheon #105

Closed
akalata opened this issue Oct 26, 2015 · 19 comments

Comments

@akalata
Copy link

akalata commented Oct 26, 2015

The docs for template_preprocess_links describes how the "is-active" class for navigation items is added dynamically via javascript for authenticated users.

I can see this behavior locally, but not in my dev environment. Identical codebase and database, both RC1 and RC2. Not sure if it's a difference in PHP version, server OS, or something else.

Localhost:
PHP 5.6.8
Apache/2.4.16 (Unix)

Drupal 8 pantheon.io:
PHP 5.5.24
nginx/1.4.7

@greg-1-anderson
Copy link
Member

You should probably submit this to the CSEs as a ticket first, with a reference to your site for preliminary investigation. (Did you run drush cr?)

Leaving this open in case it does turn out to be a platform or drops-8 issue.

@akalata
Copy link
Author

akalata commented Oct 26, 2015

Thanks Greg, last time I did that I was told to go post here since D8 wasn't yet supported. :)

@greg-1-anderson
Copy link
Member

Drupal 8 is now supported!

@ttrowell
Copy link

Internal test are revealing the "is-active" class with aggregation turned off.

@greg-1-anderson
Copy link
Member

In other words, the 'is-active' class is only missing when aggregation is turned on?

@ttrowell
Copy link

After review, I need to clarify:
Pantheon:
The active-link.js file is visibly loaded with aggregation off, but the class "is-active" is still not applied.
With aggregation off, we were not seeing the individual file, which might just be expected.

Simplytest.me:
This is a screengrab that Anna took from a simplytest.me D8 spinup:
https://www.evernote.com/shard/s167/sh/c3aa13dd-3024-4c6a-9848-e215894db546/1a5bc5c1143510cf/deep/0/

@twooten
Copy link

twooten commented Nov 3, 2015

Greg, the file "core/misc/active-link.js" is loading fine but the class called "is-active" is not being applied to links as it should be. This seems to only be a problem on Pantheon.

var activeLinks = context.querySelectorAll(selectors.join(','));
console.log(activeLinks);
var il = activeLinks.length;
for (var i = 0; i < il; i++) {
  activeLinks[i].classList.add('is-active');
}

I added the console.log() function on both localhost and a site on Pantheon to see what is returned. Each time on Pantheon, the activeLinks array is empty. On localhost the array is populated correctly.

The use case is this; If you visit http://dev-tw-d8.pantheon.io/node/3, you will see a menu of links in the left sidebar. "Home", "Page 1", and "Page 2". The link I provided is for Page 2. That link should have the class "is-active" applied to it (which just turns the text black using CSS), but the class is not being applied.

This screen shot is from my localhost and will help explain: https://www.evernote.com/l/AoreuPEyo5NA8KBbiBMsJ-4NpHsyCtaNBXg

This screen shot is from Pantheon:
https://www.evernote.com/l/Aore-4rLcM1NmZInoF0Fh7kAmGKYTfTFNDI

Aggregation being on or off has no effect.

@koppieesq
Copy link

Can you try turning off varnish & redis? These are supposed to be Good Things but they are both related to caching and might get in the way of changing the way html renders on a page. Also, it's another difference from your local environment (presumably!).

@akalata
Copy link
Author

akalata commented Nov 14, 2015

@koppieesq I'm not sure how I would disable Varnish on Pantheon? Redis is not (and has never been) enabled for this site.

According to https://pantheon.io/docs/articles/sites/varnish/, Varnish respects caching instructions sent by the site. The site is set to "no caching" and "no aggregation". Additionally, this behavior can be seen while logged in, which should negate much of Drupal's caching.

My support ticket, 47853, was closed in favor of this report. I just wish it hadn't been marked as "resolved" by the ticketing system.

@koppieesq
Copy link

Edit: Good morning @akalata ! I will look into getting that ticket reopened.

@akalata
Copy link
Author

akalata commented Nov 16, 2015

Hi @koppieesq,

  • The settings page you've screenshotted doesn't include Varnish as an option. New Relic, Solr, and Redis are not enabled. Additionally, the docs at https://pantheon.io/docs/articles/sites/varnish/ says that Varnish "static content and anonymous pages", where this issue is happening for authenticated users as well (credentials are in the support ticket)
  • The Varnish module is not included in core, and I am not using it.
  • All caching options under Performance have been disabled:
    screen shot 2015-11-16 at 2 20 53 pm
  • Cleared caches.
  • Opened Chrome (I typically use Firefox), opened incognito tab.
  • Still do not see the 'is-active' class added (for example, on http://dev-jcfs.pantheon.io/node/9)

@koppieesq
Copy link

So sorry - I realized my mistake as soon as I posted. Blame it on Monday morning. :-)

The tricky part here is that it works fine on your local environment, which means there's something different between your local env and Pantheon. The only other "quick fix" I can suggest is upgrade to D8 RC4, which came out 3 days ago.

Beyond that, I will talk to some of my colleagues and see if we can come up with an answer on our end.

@greg-1-anderson
Copy link
Member

Drupal 8.0.0-rc4 isn't running on Pantheon yet -- sorry! I'm working on it.

@greg-1-anderson
Copy link
Member

Unfortunately, is-active still not being added on #110 with the Nginx fix applied.

@greg-1-anderson
Copy link
Member

So, in the non-working case (Pantheon with Nginx), the Javascript is getting the following data for the path object:

path: Object
  baseUrl: "/"
  currentLanguage: "de"
  currentPath: "node/3"
  currentPathIsAdmin: false
  currentQuery: Object
    q: "node/3"
  __proto__: Object
  isFront: false
  pathPrefix: ""
  scriptPath: null

In a working site (local with Apache), the path variable instead looks like this:

path: Object
  baseUrl: "/"
  currentLanguage: "en"
  currentPath: "node/3"
  currentPathIsAdmin: false
  isFront: false
  pathPrefix: ""
  scriptPath: null

In other words, the page DOM is built under the assumption that clean URLs are on, but the Javascript path variable is calculated as if clean URLs are off (or maybe halfway in between -- maybe currentPath is usually 'index.php' when clean URLs are off, but I did not verify this, as it is irrelevant to our current problem.)

The next step, then, is to figure out which variables (probably http headers) the Drupal Javascript is consulting to build these strings, and determine where the spurious q=node/3 is coming from. Then, we'll need to adjust the settings that the Pantheon platform is providing so that they match Drupal's expectations for an environment running with clean URLs.

@greg-1-anderson
Copy link
Member

The 'path' component of drupalSettings is populated by system_js_settings_alter() in system.module. The currentQuery object is built via \Drupal::request(). This is just the current request, which was created in the front controller (index.php) via Request::createFromGlobals().

On the working system (local with Apache), _SERVER["QUERY_STRING"] has no value. On Pantheon with Nginx, _SERVER["QUERY_STRING"] q=node/3.

The next step is to figure out whether the query string is populated from the Nginx configuration, or if the value is injected via Pantheon -- or if it is calculated some other way.

@greg-1-anderson
Copy link
Member

It is common for Nginx configuration files for Drupal sites to be set up as shown in the example Drupal configuration page on the Nginx documentation.

The configuration setting I am skeptical about is:

    location @rewrite {
        rewrite ^/(.*)$ /index.php?q=$1;
    }

This is not used in the php location block in the above-mentioned example file, but it is used in the php location block in Pantheon's configuration. I suspect this is the problem; I will investigate further tomorrow.

@greg-1-anderson
Copy link
Member

With much thanks to @craychee, we have found a fix for this. Pantheon was using:

        location / {
            try_files $uri $uri/ @ rewrite;
        }

What is needed for Drupal 8:

        location / {
            try_files $uri $uri/ /index.php?$query_string;
        }

n.b. The config we were using is recommended for Drupal 6; the working config is recommended for Drupal >= 7.

It will take a little more work & testing to get this rolled out onto the platform, but shouldn't be much longer now.

@greg-1-anderson
Copy link
Member

Observed this working on a Drupal 8 site on the platform. Yay! To make it work, I converged the site through the admin file, and then cleared the caches on the dashboard. I expect that other existing Drupal 8 sites will automatically converge within a couple of hours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants