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

Add "Already added by other plugin/theme" #88

Open
SrJuggernaut opened this issue Jul 20, 2020 · 9 comments
Open

Add "Already added by other plugin/theme" #88

SrJuggernaut opened this issue Jul 20, 2020 · 9 comments

Comments

@SrJuggernaut
Copy link

I use elementor that already add my fontawesome pro kit. Maybe this can be added like an option alongside "use a kit", "use a cdn"

@mlwilkerson
Copy link
Member

Are you experiencing a conflict as a result of this?

This plugin has a feature for preventing conflicting loads of Font Awesome: the Conflict Detection Scanner on the Troubleshoot tab.

Part of what this plugin provides is an API on the back end--for other themes or plugins that may use it like an "icon service". So it wouldn't work for this plugin to delegate to a version of Font Awesome that's being loaded from another client. Rather, the preferred approach would be either:

  1. That this other plugin delegates its loading of Font Awesome to this plugin, or

  2. This plugin detects the conflicting Font Awesome load being introduced by other plugin, blocks that attempt, and yet loads a version that should continue to provide the services required by that other plugin.

Does it sound like I've understood your concern correctly?

@SrJuggernaut
Copy link
Author

Yes, is correct, but FontAwesome Plugin is not detecting that elementor is already loading my kit

image
image

@mlwilkerson
Copy link
Member

Oh, interesting. Thank you for following up with the screen shots.

This makes me think a couple of things:

  1. To make sure that the conflict detector is looking for kits. I can't remember whether I added that capability.

At a quick glance at the underly conflict detection code, though, it looks like I haven't add it. It would probably go right here.

The idea is: when loading that script in an isolated iframe, it creates a global window.FontAwesomeKitConfig object, then that should also be regarded as a conflict.

I'll open an internal issue to get that added.

  1. Even if the conflict detector were capable of detecting kits, I see that there's a data-fa-detection-ignore attribute on that <script>. That would cause the conflict detector to ignore that one, even if it would otherwise have detected it.

I think this indicates a bug somewhere.

If our plugin is adding that data-fa-detection-ignore, then it's a bug in our code. I did a quick check for that, and I didn't see a flaw in the logic that would account for that. (Maybe I would if/when digging deeper, though).

If Elementor is enqueuing that kit loader <script> and adding data-fa-detection-ignore, then I would regard that as an improper use of that feature on Elementor's part.

Do you have any way of debugging to figure where that's coming from?

@mlwilkerson
Copy link
Member

Update: I added a fix to the core conflict detector code to detect other kits as conflicts, and submitted it for code review.

Again, even when that gets released, I think we've still got a problem here regarding where that data-fa-detection-ignore is coming from.

@SrJuggernaut
Copy link
Author

It actually only appear when i enable the Detect Conflicts tool of the Font Awesome Plugin, so i guess is a problem with the functionality. i double checked that the Conflict detection is disabled in the kit configuration

@mlwilkerson
Copy link
Member

I've pushed PR #89 in association with this.

I also verified that if the plugin were to load a version of conflict-detection.js that looks for that window.FontAwesomeKitConfig global when testing scripts for conflicts, then it will detect this fake-kit.js in the test, as expected, resulting in this view in the conflict scanner UI:

image

@mlwilkerson
Copy link
Member

mlwilkerson commented Jul 24, 2020

As for where that data-fa-detection-ignore is coming from, it does make sense that it only shows up when you run the conflict scanner. That's when this plugin would apply those attributes: when loading under the context of conflict scanning.

It adds ignore attributes for its own scripts, as well as a bunch of standard ones, based on their WordPress resource handle.

So that makes me wonder--if Elementor isn't intentionally adding that ignore (it's still possible that it is, though I could not find evidence of that in a quick search in its open source repo)--maybe Elementor happens to be enqueuing that kit loader script with a resource handle that uses the same name as one of those that's being automatically ignored?

If you're able to run an experiment, could you try to temporarily add a line of code to cause that function to return an empty list instead? Like this:

image

What I'd like to know is whether returning an empty array here stops the data-fa-detection-ignore attribute from being added to the kit <script> that is enqueued by Elementor when the conflict scanner is running.

@SrJuggernaut
Copy link
Author

Added the empty array, the data-fa-detection-ignore still being there.
image
image

I just noticed that conflict-detection.min.js is being loaded twice. Maybe this will help to achnowlege what is happening.
image

@mlwilkerson
Copy link
Member

Now that #89 has been merged, we've got the additional conflict detection for kits in there. However, as I come back and re-read through this, I remember that there was this question as to why that data-fa-detection-ignore is being added.

It still seems that the only way that could be happening is if Elementor is intentionally adding it from their code, or perhaps their code enqueues that kit script with the same resource handle that this plugin uses, which would be font-awesome-official. That seems strange that they'd do that, so it doesn't seem likely. But those are the only two ways I can see that the ignore attribute could be applied to that script.

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

2 participants