Skip to:
Content

bbPress.org

Opened 2 years ago

Last modified 19 months ago

#3138 reopened defect

Only load core CSS+JS on bbPress pages

Reported by: DJPaul Owned by: johnjamesjacoby
Milestone: 2.7 Priority: normal
Severity: normal Version:
Component: General - Performance Keywords:
Cc:

Description

With BBP 2.6-rc3, bbPress is loading editor.min.js and bbpress.min.css on every screen on a wordpress site, even and especially those screens that have no bbPress content. It's easy to find examples in the wild, here's one: https://wordpress.org/support/

A simple change like this https://gist.github.com/anonymous/1287f6e5b925b13250e82f823513fa9b prevents those core assets being requested unnecessarily on every screen. With the shortcode support in is_bbpress() in 2.6, I've not found a problem yet doing this.

Change History (17)

#1 @johnjamesjacoby
2 years ago

  • Component changed from General to General - Performance
  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 2.7

Related: #2930.

#2 @johnjamesjacoby
2 years ago

  • Keywords needs-testing removed
  • Milestone changed from 2.7 to 2.6
  • Owner set to johnjamesjacoby

I can imagine how this might break (if people are calling the bbp_enqueue_ functions directly or out of order) but, IMO, that's starting to void the warranty a bit, no different than what would happen in WordPress already.

I'm comfortable moving this to 2.6 and addressing it now.

#3 @johnjamesjacoby
2 years ago

  • Resolution set to fixed
  • Status changed from new to closed

In [6641]:

Scripts: Do not enqueue if not is_bbpress().

This reduces the number of external scripts that are enqueued on non-bbPress pages. Put another way, it makes sure that bbPress styles & scripts are only loaded on pages where bbPress content is known to exist.

Fixes #3138. Props DJPaul.

Last edited 2 years ago by netweb (previous) (diff)

#4 @johnjamesjacoby
2 years ago

  • Milestone changed from 2.6 to 2.7
  • Resolution fixed deleted
  • Status changed from closed to reopened

This change exposed a problem with an advanced edge-case, where custom templates pulling in queries (or their own parts) no longer get the benefit of the default styling being included with it, a situation employed on BuddyPress.org and bbPress.org, and likely other places where people were breaking all the rules.

Reopening this and moving it to 2.7. Not 100% confident I want to revert this yet, since we can probably wait and see who (if anyone) runs into this, and quickly revert and put out a point release.

#5 @Robin W
21 months ago

Scripts: Do not enqueue if not is_bbpress().

2.6 rc5 on twenty fifteen theme

On the support forums user has issues with css not loading

  1. Where a shortcode is used on the homepage via code, css doesn't load.

<?php echo do_shortcode("[bbp-forum-index]"); ?>

ISSUE 2 now confirmed as a site specific issue and fixed !!
2. It doesn't load either when in profile>topics or replies eg

http://jockjams.com/forums/users/bobby/topics/

ISSUE 2 now confirmed as a site specific issue and fixed !!

bbpress support forum :

https://bbpress.org/forums/topic/bbp-topic-index-shortcode-displaying-without-styles/

Last edited 21 months ago by Robin W (previous) (diff)

#6 @Robin W
21 months ago

Further test by user

shortcode works when used directly in a wordpress page, but not if used programmatically.

Last edited 21 months ago by Robin W (previous) (diff)

#7 @Robin W
21 months ago

  • Milestone changed from 2.7 to 2.6
  • Type changed from enhancement to defect

changed this back to 2.6 as it is affecting rc5 and logging as a defect. If I should be opening a new ticket, please advise.

#8 @Robin W
21 months ago

I have also found that none of the shortcodes in my additional plugins (eg bbp style pack and bbp private groups) which call bbpress functions and templates display correctly as the bbpress.css is not loaded !

@netweb - how do I get my plugin shortcodes to work with 2.6 ?

#9 @johnjamesjacoby
21 months ago

We would need to be crafty with how we detect that shortcodes have been called, and which shortcodes should load bbPress styling.

This is because there is no “namespace” or “source” or “context” for them to know if they are bbPress or anything else.

We could, say, listen for shortcodes that start with bbp- and always enqueue, and that seems OK I guess, but it’s pretty hokey.

The other problem is that these dynamic-type calls happen mid-page, so we are just-in-time enqueueing assets in such a way that page caching and remote assets like CDNs won’t really have much luck doing what they’re intended to do.

We can do it, but it may introduce other edge-case issues.

Last edited 21 months ago by johnjamesjacoby (previous) (diff)

#10 follow-up: @Robin W
21 months ago

so can we revert to just loading bbpress.css in all cases please ? - the file is small, and frankly most sites are image rich nowadays and a 32kb min.css file is a fraction of any image on a home page !

Otherwise

  1. your shortcodes don't work if called from code
  2. My shortcodes won't work unless

a)I either try and enqueue from within the shortcode if that is possible (no idea how I do that) and which may well mean that the bbpress.css is then loaded twice (??) and makes the site slower, or otherwise
b)I have to have a clone of bbpress.css and load that in my plugin, which definitely makes the css have to load twice !!

And in all honesty if only loading css on bbpress pages is designed to make sites run faster, then only loading them on bbpress pages will definitely make forums look slower as they have to load css - which really isn't to bbpress advantage :-)

Last edited 21 months ago by Robin W (previous) (diff)

#11 in reply to: ↑ 10 @Destac
19 months ago

Loading the CSS on every page is not ideal for performance it just isn't relying on the user's browser cache is a very bad idea, often times people are combining the CSS/JS with some various tools Autoptimize, W3 ETC where the file URL will be different on each page. When designing website's to load quickly you don't want to fall into the idea of just oh the browser will cache it. You want to design website's to load quickly as if the visitors were brand new and not rely on browser caching to make your site fast.

However, figuring out how themes are loading the CSS is rather important. Some of the themeforest themes for instance often integrate in Visual Composer so we would need to account for those use cases. Some site's incorporate the forum-index as their website's homepage and we have to account for that.

There won't be a one-size fit's all solution to this imo. It's likely going to come down to one of two scenarios.

  1. Nothing is changed and everything continues to work.
  2. Things break and theme developers either need to fix it or there will be problems for their users.

I also want to address has this been tested for compatibility with BuddyPress as it incorporates forums on profiles & group pages. Often times when working in those environments I don't dequeue the styles on BuddyPress pages as it would lead those forums unstyled. I haven't tested it yet but I want to know if someone else has before I go and try it.

#12 @Robin W
19 months ago

But isn't that the idea of CSS, that it cascades so just gets loaded once. loading 32kb at the start doesn't seem to be a large overhead, and increasingly site managers are using child themes without being that technical, as they just google code to go into your files.

As I say at the moment 2.6 is broke in the sense that it fails to load the css unless it thinks it is on a bbpress page, so my plugins which rely on css loading after bbpress css don't work on it, nor does anyone using shortcodes within code.

Similarly anyone using css in a child theme which overwrites bbpress css will find that it no longer does, as their code loads at the start, and bbpress then overwrites it when it loads on a page.

If someone knows how to overcome this, that would be great - but at the moment this looks like breaking the look of thousands of site when 2.6 is released, which is not going to be a PR success story.

Last edited 19 months ago by Robin W (previous) (diff)

#13 @johnjamesjacoby
19 months ago

I think it’s OK to revert this for 2.6 since it caused known breakage, and continue investigating how to improve this for future versions.

I’d like us to be as efficient as possible when it’s possible, and it’s OK if we haven’t completely figured that out yet.

#14 @Robin W
19 months ago

sounds like a plan !

#15 @Destac
19 months ago

Agreed best solution at the moment!

#16 @johnjamesjacoby
19 months ago

In 6795:

Templates: revert r6641 due to reports of styling breakage.

This reverts back to the 2.5-type behavior of always making sure bbPress styling & scripts are available on all theme-side pages.

We will revisit this enhancement again in a future release.

See #3138. Props robin-w.

#17 @johnjamesjacoby
19 months ago

  • Milestone changed from 2.6 to 2.7

Moving to 2.7 to research further after 2.6.

Note: See TracTickets for help on using tickets.