hofff / contao-calendarfield

GNU Lesser General Public License v3.0
18 stars 3 forks source link

Duplicate assets domain and jQuery #33

Closed seaneble closed 7 years ago

seaneble commented 7 years ago

The following markup is generated if there is a separate assets domain in the root page settings:

<link rel="stylesheet" href="https://code.jquery.com/ui/1.11.4/themes/smoothness/jquery-ui.css">
<script src="//cdn.domain.tld/assets/js/250f6dcf15eb.js"></script>
<script src="//cdn.domain.tld///cdn.domain.tld/assets/jquery/ui/1.11.4/jquery-ui.min.js"></script>
<script src="//cdn.domain.tld///cdn.domain.tld/assets/hofff/calendarfield/jquery.ui.datepicker/1.11.4/jquery.ui.datepicker.min.js"></script>
<script src="//cdn.domain.tld///cdn.domain.tld/assets/hofff/calendarfield/jquery.ui.datepicker/1.11.4/i18n/jquery.ui.datepicker-de.js"></script>

Two issues with that code:

  1. Why is jQuery included twice?
  2. Why is the assets domain printed twice for some assets? They obviously result in 404 errors
cliffparnitzky commented 7 years ago

for 1. : i don't understand. what is meant ???

for 2. : the value of TL_ASSETS_URL ist added here: https://github.com/hofff/contao-calendarfield/blob/master/CT_ROOT/system/modules/hofff_calendarfield/forms/FormCalendarField.php#L66

But the Contao/Controller only checks URLs with protocoll : https://github.com/contao/core/blob/master/system/modules/core/library/Contao/Controller.php#L1805

So, please add a protocoll to youre assets url. Maybe we also have to fix this in the code. @frontendschlampe What do you think?

frontendschlampe commented 7 years ago

it's very useful, to add an assets url without protocol - so I think we have to fix this

seaneble commented 7 years ago

Actually, if you enter a protocol within the global settings, it is stripped by Contao. This does not happen in the page settings.

cliffparnitzky commented 7 years ago

@seaneble To fix this issue, we have to clearify yout question 1. What exactly is meant? Where is jQuery included twice in your code example?

seaneble commented 7 years ago

See line 1 and line 3 of my snipped.

cliffparnitzky commented 7 years ago

Hmm, the 1. ist CSS and the 3. is JS ... seems to be okay.

seaneble commented 7 years ago

Well, that's embarrassing. I'm sorry. You're right, of course.

cliffparnitzky commented 7 years ago

Great. So, finally we only have to fix the 2. point (probleme with assets url). Would be planned asap.

cliffparnitzky commented 7 years ago

This is only a probleme in Contao 3.5 ... with Contao 4.4 the code works fine ... nevertheless the variable TL_ASSETS_URL is not needed and should be removed