Hmm, we would still not be able to change SqueezeBox to something else before Joomla 4.0 where we can break B/C. So I'm not sure if this PR really achieves anything in the end.
Instead of relying on JHtmlBehavior::modal() one can also use the modal feature from Bootstrap (JHtmlBootstrap::modal()).

This is not an issue of breaking B/C and this is not about changing the default modal provider in Joomla. This is future-proofing and giving more power to users. Suppose I, as a user, want to write a template for my site that uses some other modal dialog provider. I want to do this by overriding JHtmlBehavior::modal() so that I have a consistent interface for using modals and any modals used by built-in parts of Joomla will also use whatever I'm using. I can't really do that now because calls to SqueezeBox.close() are basically hard coded. Sure, they're in template files which, in theory, can be overridden but it's kind of silly to do that just to change a single line.

Besides that, there are some things that actually can't be overridden easily. For example, if I use the editor on site (instead of the admin) and I click the image button, a modal will show up with the com_media images view. This template is one of the places where SqueezeBox.close() is being called and it can not be overridden by a site template, it must be overridden by an admin template. So it's a huge pain to fix a tiny problem.

The Bootstrap modal is not really equivalent and not really a replacement for SqueezeBox without adding quite a bit of custom code. Besides, for consistency, you'd want to use only one or the other and JHtmlBehavior::modal() can't really be avoided. So then, maybe my solution is to override JHtmlBehavior::modal() with some fancy way of using the Bootstrap modal. This is definitely doable but I'll still have various buttons trying to close SqueezeBox even though it doesn't exist.

A little off topic but, since you brought up JHtmlBootstrap, that class also has a lot of issues such as a lot of cases of jQuery being used without the ready function. I wonder if anyone else has noticed that problem.

If it's to allow better template overrides, then you should also imho move the javascript loading to an overrideable place (like using JLayout or loading the JS from a file in media folder).

Also it will only solve this for core extensions. Any 3rd party extension probably still uses SqueezeBox.close(). That's why I think it doesn't solve a lot.
And I want to avoid that in future someone thinks that he can easily change SqueezeBox to something else without breaking anything. Because it would work fine in core but break 3rd party extensions.
Maybe you can add a comment to avoid that?

All JHtml functions are already easily overrideable using JHtml::register() though. So I don't think it's necessary to also override them in a layout for something simple like this.

There is a solution that will solve this for the special case that you mention. It's not necessary to include it here as executable code but a comment with details on how to do it would be helpful. I'll add it soon.

All JHtml functions are already easily overrideable using JHtml::register() though. So I don't think it's necessary to also override them in a layout for something simple like this.

I know it's possible. But you need a system plugin to do that. Our goal is to allow all output be easily overriden by a template, without the need for plugins or other coding.
@phproberto is working on that together with his frontender working group. It may be worth a thought 😄

I like layouts a lot. I really do. But I hope they don't become the hammer that makes everything look like a nail. I think layouts are best when used for rendering a block of html or, in some cases, javascript. The JHtml functions still seem to me like the best place to load javascript and css files but maybe that's because I don't shy away from making my own plugins. If the issue is that they're not easy enough to override, why don't we look at ways to make that easier? Suppose there was a designated place inside your template directory where you could place some functions that would automatically override JHtml functions? I think it makes more sense than layouts for everything.

Suppose there was a designated place inside your template directory where you could place some functions that would automatically override JHtml functions? I think it makes more sense than layouts for everything.

Valid point of course.

@okonomiyaki3000 Can you update this PR? Since it has merge conflicts it can't be tested properly. Thanks.

Yes, definitely a leftover. @Bakual can you please remove that line? I am on vacation this week and it's improbable I'll have enough time to open the laptop at all :)

Removed the comment.
@okonomiyaki3000 Means you need to rebase your PR again. Sorry for the trouble.

OK, done.

@okonomiyaki3000

Github say that this pull request contains merge conflicts that must be resolved. Can you have a look into it?

👌

thanks @okonomiyaki3000 tested successful

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/3918.

@test Success

Moving to RTC since we have 2 successful tests.

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/3918.

This has to be updated to staging

Doing some cleanup on a few lines that are not mine so that Travis will pass. I've noticed a few things. First, the PHPCS standard works fine for most code but it has some issues with template code. Often, if you write something like

<?php foreach ($someArray as $item) : ?>
    <div>etc...</div>
<?php endforeach; ?>

You may see some complaints about blank lines found at the start of control structures and etc.

Also, there appears to be an extra bootstrap.endSlide in this template. I'm not going to touch it because it's not relevant to this PR and anyway I might have overlooked something but it does not appear to match any bootstrap.addSlide.

@okonomiyaki3000 Could you make that final change we discussed and update to the latest staging? After that I can merge this :)

Ah, you want it in this pr? It's not really related but, ok. I can't probably do it till next week though. Now it's お正月休み...

@okonomiyaki3000 The line is already touched by your PR, so I would say it is fine. No worries, next week is fine. Thank you and Happy holidays :)

I may need to rebase this.

Looks mergeable now.

@test Success

@test
Fails. Menu Manager=>New Menu Item
Selecting a menu item type in the modal does not close the modal anymore when item clicked.

It's my habit to make strings js safe by using json_encode but it uses double quotes which we do not want in this case. It should be safe to use a non-json encoded string here since it's the output of base64_encode and that should not contain any problematic characters. In any case, that's how the original code was working.

OK now. Thanks.

@test All good here as well.

So the RTC state is good here. 😄