Skip to content

Commit

Permalink
[fc] Repository: plone.base
Browse files Browse the repository at this point in the history
Branch: refs/heads/main
Date: 2022-09-07T15:36:52Z
Author: Rudd-O (Rudd-O) <rudd-o@rudd-o.com>
Commit: plone/plone.base@23708ce

Fix folder_contents being broken with VHM and Multilingual

When Multilingual is enabled and VHM is enabled too, serving any folderish's `folder_contents` under a language folder as a top level domain name is currently broken (if `en.site.com/archives` is VHMed to `PloneSite/en/archives` then `@@qsOptions` URL is `/archives/@@qsOptions` instead of being `/@@qsOptions` as it should be.

The fundamental problem is that the function `get_top_site_from_url(...)` is failing to retrieve the actual top site because it can't find a site in the traversal structure, so it picks the last folderish as the site, which is obvs broken because `/@@qsOptions` only works on site roots, not on folderish content.

This code makes it so that any site found in the structure is returned as the top site from the URL, but if no top site is found in that traversal, the actual site from `getSite()` is returned instead, which is the correct behavior based on how the function is named, and actually fixes the `/@@qsOptions` conundrum.

Files changed:
M src/plone/base/utils.py
Repository: plone.base

Branch: refs/heads/main
Date: 2022-09-07T16:54:47Z
Author: Rudd-O (Rudd-O) <rudd-o@rudd-o.com>
Commit: plone/plone.base@4f88a01

Add towncrier announcement.

Files changed:
A news/18.bugfix
Repository: plone.base

Branch: refs/heads/main
Date: 2022-09-07T17:42:57Z
Author: Rudd-O (Rudd-O) <rudd-o@rudd-o.com>
Commit: plone/plone.base@49a80b2

Include the topmost accessible object in the subsites list.

Files changed:
M src/plone/base/utils.py
Repository: plone.base

Branch: refs/heads/main
Date: 2022-09-07T18:03:57Z
Author: Rudd-O (Rudd-O) <rudd-o@rudd-o.com>
Commit: plone/plone.base@60dcd49

Add comment to give context w.r.t p.a.multilingual.

Files changed:
M src/plone/base/utils.py
Repository: plone.base

Branch: refs/heads/main
Date: 2022-09-07T18:19:35Z
Author: Rudd-O (Rudd-O) <rudd-o@rudd-o.com>
Commit: plone/plone.base@3199ddb

Remove redundant comment.

Files changed:
M src/plone/base/utils.py
Repository: plone.base

Branch: refs/heads/main
Date: 2022-09-07T19:16:33Z
Author: Rudd-O (Rudd-O) <rudd-o@rudd-o.com>
Commit: plone/plone.base@653bb70

Correct tests.

Files changed:
M src/plone/base/utils.py
Repository: plone.base

Branch: refs/heads/main
Date: 2022-09-07T19:31:09Z
Author: Rudd-O (Rudd-O) <rudd-o@rudd-o.com>
Commit: plone/plone.base@b1ff114

Update tests to cover the test case that this PR fixes.

Files changed:
M src/plone/base/tests/test_utils.py
Repository: plone.base

Branch: refs/heads/main
Date: 2022-09-07T21:29:14Z
Author: Rudd-O (Rudd-O) <rudd-o@rudd-o.com>
Commit: plone/plone.base@991cd48

Clarify comment in this crucial piece of code.

Now that tests are passing and the functionality of the site is working perfectly, I give myself some time to give a better rationale for this change as comments, for future me and future Plonistas.

Files changed:
M src/plone/base/utils.py
Repository: plone.base

Branch: refs/heads/main
Date: 2022-09-07T21:49:23Z
Author: Rudd-O (Rudd-O) <rudd-o@rudd-o.com>
Commit: plone/plone.base@a80196c

Add warning to people who might want to use this function incorrectly.

Files changed:
M src/plone/base/utils.py
Repository: plone.base

Branch: refs/heads/main
Date: 2022-09-07T21:51:58Z
Author: Rudd-O (Rudd-O) <rudd-o@rudd-o.com>
Commit: plone/plone.base@7d7da99

Change oneliner description of method, since it was misleading to begin with.

Files changed:
M src/plone/base/utils.py
Repository: plone.base

Branch: refs/heads/main
Date: 2022-09-08T16:45:29Z
Author: Rudd-O (Rudd-O) <rudd-o@rudd-o.com>
Commit: plone/plone.base@1199d72

Merge pull request #18 from Rudd-O/main

Fix folder_contents being broken with VHM and Multilingual

------------------------------

When Multilingual is enabled and VHM is enabled too, serving any folderish's `folder_contents` under a language folder as a top level domain name is currently broken (if `en.site.com/archives` is VHMed to `PloneSite/en/archives` then `@@qsOptions` URL is `/archives/@@qsOptions` instead of being `/@@qsOptions` as it should be.

The fundamental problem is that the function `get_top_site_from_url(...)` is failing to retrieve the actual top site because it can't find a site in the traversal structure, so it picks the last folderish as the site, which is obvs broken because `/@@qsOptions` only works on site roots, not on folderish content.

This code makes it so that any site found in the structure is returned as the top site from the URL, but if no top site is found in that traversal, the actual site from `getSite()` is returned instead, which is the correct behavior based on how the function is named, and actually fixes the `/@@qsOptions` conundrum.

# Reproducer

Odd intersection of circumstances to reproduce bug:

1. Create classic `Plone` site.
2. Enable Multilingual with `en-US` or some other language.
3. Verify visiting `/Plone` redirects to `Plone/en-US` or your language folder of choice.
4. VHM your site so that `/Plone/en-US` is accessible as `/` (a standard, supported Multilingual rewrite configuration typically used by Ploners for language-code subdomain hosting).  `Plone/en-US/VirtualHostRoot` is the relevant VHM rewrite.
5. Visit the site.  Verify that site is accessible as `/`.
6. Create folder `xxx` on that location.
7. Visit folder `xxx` on that location.
8. Click on *Folder contents* in the editor toolbar.
9. Verify that the folder contents screen for `xxx` loads, but the actual listing remains empty.
10. Verify that there is an HTTP 404 for `@@qsOptions` in the network panel of your browser inspector.

# Rationale

This code makes the method sometimes return a content object in some cases, rather than an actual site root.

Why?

All callsites (see below) need a "root object" *from the perspective of the browser* to compute a root and generate various URLs for their own purposes, because all their purposes are XHR browser stuff (fill lists of objects and search results, chiefly).  This method I'm fixing determines the "root" used for that tree which is used in related items, image insertion, and folder contents browsing, and if that "root" is incorrect / does not correspond to what the browser sees as "root", **nothing works**.

Needless to say, this method should never be used to traverse anything server-side.

# Quality control

## Callsites 

My omelette is big.  There are not that many uses, and all its uses currently broken are fixed by this patch.  I count two uses:

1. `plone.app.content.browser.contents`:`ContentsBaseAction` / `get_options` / `ContextInfo.call`
2. `plone.app.widgets`:`get_relateditems_options`

## Unit and regression testing

All current unit tests pass now.  I have added a test case to cover the "root URL object is not actually a site" case, which was never tested before.  In addition to that, I have tested this with a local build (most up to date code) and here is what I can verify:

* Related items works now!
* Image insertion works as well.
* Browsing folders in folder contents works too.

So pretty much everything that was broken before, is no longer broken thanks to this modest change.  And that is the complete manual regression test of all users of this code.

Files changed:
A news/18.bugfix
M src/plone/base/tests/test_utils.py
M src/plone/base/utils.py
  • Loading branch information
Rudd-O committed Sep 8, 2022
1 parent edaa7f5 commit beb9bea
Showing 1 changed file with 210 additions and 15 deletions.
Loading

0 comments on commit beb9bea

Please sign in to comment.