Why not just submit a PR to Sitemesh 3 if any fixes are needed? Sitemesh 2.6.x was just a quick patch for a codebase abandoned in 2009. If you were to revert to Sitemesh 2, the proper course of action would be Sitemesh 2.7.x

@codeconsole It's a timing problem. We're running out of time to get these issues fixed so going to 2.6.0 is a stop gap since it makes us compatible with 6. I think it's reasonable to consider 2.7.0 for 7.1. Especially, if we have help or volunteers to do the upgrade. The only reason we're trying to move forward here is because of the blocker it's had on the 7 release.

If I remember correctly, going back to 2.x has major negative consequences. It basically compiles Sitmesh 2.x code into everything so any future upgrades will result in breaking every plugin.

Can you please help me understand what would cause a plugin to break? I assume it's the GSP compiled logic? I think we may have to deal with that since it's the same as 6.x. We're open to suggestions on how to reduce this. The core issue is the architecture difference and not integrating into the Spring DispatcherServlet.

no, it has to do with references to sitemesh logic in traits that are compiled into a lot of plugin classes. When you try to use them in a future version, you get an exception. The only workaround at that point is re-releasing every plugin.

I don't see that on first glance of this PR. If you can take a look at this review to help me find that, I think decoupling by introducing interfaces/intermediate classes in Grails so in the future we can remove sitemesh2 is very reasonable.

Well, you can start by looking at your changes to the ResponseRenderer trait and look at all the locations that gets compiled into.

I don't see the need to push this back into the dark ages. Don't most of these issues have workarounds? Why not just use decorator chaining if you want to apply multiple layouts?

Almost all of this feedback was on the original code that I restored without changing. I agree with cleaning it up, but I originally hadn't to reduce the potential for problems. My last commit contains all of the feedback.

I had to revert the cast that shows an error in intellij. Otherwise, a large amount of tests fail with a cast exception.

I think a more rational decision would be to fork 8 before this is merged and keep 8 moving forward.

I had to revert the cast that shows an error in intellij. Otherwise, a large amount of tests fail with a cast exception.

That is interesting considering the cast is made inside an if (content instanceof StreamCharBuffer) condition.
Doing a proper cast seems to work better:

gspGrailsLayoutPage.pageBuffer = (StreamCharBuffer) content

Thanks @matrei , that cast seems to work.

I'm going to restructure the commits in this PR so that it's easier to revert sitemesh 2. This way we can keep branches merged cleanly.

I'm going to open another PR and split this one between just the sitemesh2 change and all of the clean up / test pollution fixes / and test case restores

This PR now is only a single commit with only the sitemesh2 revert.

#14876 contains all of the feedback / cleanup related commits prior to the sitemesh revert.

This will allow us to revert one commit to have sitemesh3

Had to rebase this PR to pickup the other upgrade doc changes. No further changes were made.