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.

Sign up for GitHub

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

Jump to bottom

Tag the responses within ContentModule #9

Merged
merged 1 commit into from Aug 21, 2018
Merged

Tag the responses within ContentModule #9

merged 1 commit into from Aug 21, 2018

Conversation

richardhj
Copy link
Member

Tag the response with a contao.tb.tl_content tag as well. While the response already has a tl_module tag, the tl_content tag is necessary since the content element can hold a custom CSS ID which requires the response to become invalidated post change.

How to reproduce

  1. Activate shared caching
  2. Create module (e.g. news list)
  3. Include module (ContentModule)
  4. Request page in frontend
  5. Change cssId of the content element including module
  6. Reload the page; the page should be expired.

I am not experienced with caching, and I came up with this kind of heuristically, so please evaluate.

@Toflar
Copy link
Member

Toflar commented Aug 20, 2018

Perfect one!

@Toflar
Copy link
Member

Toflar commented Aug 20, 2018

Ah, hang on 😄 The news list should get the content element using Controller::getContentElement() no?

@richardhj
Copy link
Member Author

I did not test with news list but with a MetaModel list in my case. Sorry for this malicious example :-D
I just now verified with a "page image" module.
Every module (but custom html) with $this->cssID should be affected.

@Toflar
Copy link
Member

Toflar commented Aug 20, 2018 •
edited

Ah, I see. This PR is fine. It is not tagged because ContentModule::generate() overrides ContentElement::generate() and does not call parent::generate().

@richardhj
Copy link
Member Author

👍

@richardhj
Copy link
Member Author

Just noticed: Without this fix, you are not able to change the included module of the content element. For the same reasons.

@leofeyer
Copy link
Member

@Toflar Should we add this in Contao 4.6?

@Toflar
Copy link
Member

Toflar commented Aug 20, 2018

Yes, this is a bugfix.

@leofeyer leofeyer added this to the 4.7.0 milestone Aug 20, 2018
@Toflar
Copy link
Member

Toflar commented Aug 20, 2018

Or well. Generally speaking adding tags should be allowed in bugfix releases anyway because that doesn't really hurt.

@leofeyer leofeyer changed the base branch from master to 4.6 August 20, 2018 14:31
@leofeyer leofeyer changed the base branch from 4.6 to master August 20, 2018 14:31
@leofeyer
Copy link
Member

@richardhj Can you rebase the PR against the 4.6 branch then please?

@leofeyer leofeyer modified the milestones: 4.7.0, 4.6.0 Aug 20, 2018
@richardhj
Copy link
Member Author

The branch and diff view is clean now, isn't it?

@leofeyer leofeyer changed the base branch from master to 4.6 August 20, 2018 15:10
@leofeyer
Copy link
Member

Unfortunately not. I have changed the base of your PR now, so you see the problem.

@richardhj
Tag the responses within ContentModule
596c488 
Tag the response with a `contao.tb.tl_content` tag as well. While the response already has a `tl_module` tag, the `tl_content` tag is necessary since the content element can hold a custom CSS ID which requires the response to become invalidated post change.
@richardhj
Copy link
Member Author

I have changed the base of your PR now, so you see the problem.

Ah. Didn't noticed the changing back. I fixed the branch.

@leofeyer
Copy link
Member

Thank you very much. 👍

@leofeyer leofeyer merged commit ad4194a into contao:4.6 Aug 21, 2018
@CMSworker CMSworker mentioned this pull request Sep 21, 2018
Closed
@ghost ghost mentioned this pull request Feb 28, 2020
Closed
leofeyer pushed a commit that referenced this pull request Aug 4, 2020
@qzminski
Fix a potential error if the URL has percentage in it (see #619)
0a7b13d 
Description
-----------

With @Toflar we have discovered by accident that if a URL is double-encoded (for some reason, doesn't matter) the Contao's `RouteProvider` will eventually throw an error trying to query a database.

```
URL original: drachenlochmuseum-v%25c3%25a4ttis.html
URL decoded: drachenlochmuseum-v%c3%a4ttis.html
URL decoded 2nd time: drachenlochmuseum-vättis.html
```

The decoded URL is used in the database query and that fails because the database driver would like to replace wildcards `%c` with parameters that were not provided.

Stack trace:

```
Exception: Too few arguments to build the query string
#27 vendor/contao/core-bundle/src/Resources/contao/library/Contao/Database/Statement.php(304): replaceWildcards
#26 vendor/contao/core-bundle/src/Resources/contao/library/Contao/Database/Statement.php(249): execute
#25 vendor/contao/core-bundle/src/Resources/contao/library/Contao/Model.php(1102): find
#24 vendor/contao/core-bundle/src/Resources/contao/library/Contao/Model.php(973): findBy
#23 vendor/contao/core-bundle/src/Framework/Adapter.php(38): __call
#22 vendor/contao/core-bundle/src/Routing/RouteProvider.php(493): findPages
#21 vendor/contao/core-bundle/src/Routing/RouteProvider.php(88): getRouteCollectionForRequest
#20 vendor/contao/core-bundle/src/Routing/LegacyRouteProvider.php(43): getRouteCollectionForRequest
#19 vendor/symfony-cmf/routing/src/NestedMatcher/NestedMatcher.php(141): matchRequest
#18 vendor/contao/core-bundle/src/Routing/Matcher/LegacyMatcher.php(69): matchRequest
#17 vendor/symfony-cmf/routing/src/DynamicRouter.php(271): matchRequest
#16 vendor/symfony-cmf/routing/src/ChainRouter.php(188): doMatch
#15 vendor/symfony-cmf/routing/src/ChainRouter.php(158): matchRequest
#14 vendor/symfony/http-kernel/EventListener/RouterListener.php(115): onKernelRequest
#13 vendor/symfony/event-dispatcher/EventDispatcher.php(212): doDispatch
#12 vendor/symfony/event-dispatcher/EventDispatcher.php(44): dispatch
#11 vendor/symfony/http-kernel/HttpKernel.php(126): handleRaw
#10 vendor/symfony/http-kernel/HttpKernel.php(67): handle
#9 vendor/symfony/http-kernel/Kernel.php(198): handle
#8 vendor/symfony/http-kernel/HttpCache/SubRequestHandler.php(85): handle
#7 vendor/symfony/http-kernel/HttpCache/HttpCache.php(448): forward
#6 vendor/symfony/framework-bundle/HttpCache/HttpCache.php(57): forward
#5 vendor/symfony/http-kernel/HttpCache/HttpCache.php(420): fetch
#4 vendor/contao/manager-bundle/src/HttpKernel/ContaoCache.php(46): fetch
#3 vendor/symfony/http-kernel/HttpCache/HttpCache.php(317): lookup
#2 vendor/symfony/http-kernel/HttpCache/HttpCache.php(192): handle
#1 vendor/friendsofsymfony/http-cache/src/SymfonyCache/EventDispatchingHttpCache.php(98): handle
#0 web/app.php(58): null
```

Commits
-------

8ae2582 Fix a potential error if the URL has percentage in it
8caaf25 Fix unit tests
509f762 Correctly encode the page aliases
leofeyer pushed a commit that referenced this pull request Nov 8, 2021
@Toflar
Support canonical URLs in the front end (see #3616)
a48f312 
Description
-----------

This includes most of the features discussed in #3097 (comment)

Commits
-------

6a63d40 Added support for canonical link handling to html head bag
8849810 Allow to set canonical url manually
bfe8f29 Added support for parameter wildcards
c9bde3e Added back end integration
e737edf Bye $GLOBALS['TL_NOINDEX_KEYS']
a02c18f Implemented output of rel="canonical"
902d705 Different field order
72b18be Fixed tests
b3ba34d Merge branch '4.x' into feature/canonical
a9634a5 Update core-bundle/src/Resources/contao/languages/en/tl_page.xlf

Co-authored-by: Fritz Michael Gschwantner <fmg@inspiredminds.at>
47b0409 Update core-bundle/src/Resources/contao/templates/frontend/fe_page.html5

Co-authored-by: Fritz Michael Gschwantner <fmg@inspiredminds.at>
a58098b Update core-bundle/src/Routing/ResponseContext/HtmlHeadBag/HtmlHeadBag.php

Co-authored-by: M. Vondano <m-vo@users.noreply.github.com>
22d8d49 Make sure the custom canonical URI can be retrieved as is
627ab48 Normalize custom URI as well
9122bea Fixed CI
e6f6fbd Merge branch '4.x' into feature/canonical
2dd6e97 Merge branch '4.x' into feature/canonical
06ea58a CS
1acca17 Use the request object instead of the Environment class
4ac1409 CS
4eb4a05 Also add the base path
e8c7d40 Merge pull request #7 from leofeyer/feature/canonical-without-environment

Use the request object instead of the Environment class
f595819 Merge branch '4.x' into feature/canonical
bdfcb55 Disable the canonical fields instead of adding them dynamically (#9)
cd936ea Remove the block around the canonical tag (#10)
@andyKempf andyKempf mentioned this pull request Mar 30, 2022
Closed
leofeyer added a commit that referenced this pull request Nov 29, 2023
@leofeyer @aschempp
Show the back end header on scroll-up (see #6569)
85fb139 
Description
-----------

Thank you @ausi (see https://codepen.io/ausi/pen/rNPrmGX).

Commits
-------

b08000e Show the back end header on scroll-up
9bbbe07 Add a Stimulus controller for scroll events (see #9)
6b9255e Only make the header fixed on scroll-up
355be22 Clean up
713fc20 Add a configurable threshold value
fb6e6d9 Add the CSS class to the <html> element

Co-authored-by: aschempp <andreas.schempp@terminal42.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Toflar Toflar Toflar approved these changes

Assignees

@richardhj richardhj

Labels
feature
Projects
None yet
Milestone
4.6
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@richardhj @Toflar @leofeyer