Problem/Motivation

Looks like the admin pages are including the js library too. It would be nice to exclude them to avoid the extra requests.

Proposed resolution

Do path or admin page exclusions similar to xhprof or other modules.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#15 2746695-15.patch3.05 KBjoelpittet
#8 2746695-8.patch2.13 KBjoelpittet
#6 2746695-6.patch2.13 KBjoelpittet

Comments

joelpittet created an issue. See original summary.

cilefen’s picture

cilefen commented
Related issues: +#2543116: JavaScript is loading when no input formats have the filter applied
cilefen’s picture

cilefen commented
Version: 7.x-2.x-dev » 8.x-2.x-dev
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Do you have a preference @cilefen which direction this issue takes? I don't mind rolling a patch

cilefen’s picture

cilefen commented

@joelpittet I am open to anything.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Active » Needs review
StatusFileSize
new 2746695-6.patch2.13 KB

Ok here's something similar to what I did with olark module.

Status: Needs review » Needs work

The last submitted patch, 6: 2746695-6.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
StatusFileSize
new 2746695-8.patch2.13 KB
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs work » Needs review

Typo from copy/paste

cilefen’s picture

cilefen commented
Status: Needs review » Needs work 
+++ b/mathjax.module
@@ -25,6 +25,12 @@ function mathjax_help($route_name, RouteMatchInterface $route_match) {
+  if (!$config->get('enable_for_admin') && \Drupal::service('router.admin_context')->isAdminRoute()) {

The "hello world" library test is on an admin page. We should special case that.

staceroni’s picture

staceroni commented

The site I am using this on has 15 content types, but the Mathjax library only needs to be used in 1 content type.

The ability to restrict the Mathjax library to certain paths would certainly be helpful.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

@staceroni Have a look at this patch: #3016329: Conditionally attach MathJax libraries and markup It only renders if there is content!

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

@cilefen I can't see a "hello world" library test, what is the route for that?

cilefen’s picture

cilefen commented

@joelpittet https://git.drupalcode.org/project/mathjax/blob/8.x-2.x/mathjax.routing....

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs work » Needs review
StatusFileSize
new 2746695-15.patch3.05 KB

Ah good catch but we already exclude implicitly by manually adding the libraries on that form.
https://git.drupalcode.org/project/mathjax/blob/8.x-2.x/src/Form/Mathjax...

Here's an update with the config actually saving and an update hook to preserve BC but leaving new installs having it disabled.

  • joelpittet committed a0d08f4 on 8.x-2.x 
    Issue #2746695 by joelpittet, cilefen, staceroni: Allow path and admin...
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Fixed

Committed to the dev branch

  • joelpittet committed 9baf3ec on 3.0.x 
    Issue #2746695 by joelpittet, cilefen, staceroni: Allow path and admin...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.