Another small follow-up from #2068471: Normalize Controller/View-listener behavior with a Page object. That patch introduced renderers for HtmlFragments and HtmlPages, but made the silly mistake of making them the same service. This patch corrects that mistake by splitting them into two separate services, which is what we should have done in the first place. No other changes.
This was already discussed in the course of #2028749: Explore addressable block rendering and we decided it was a good idea, as we expect modules to override the page renderer WAY more often than the fragment renderer; it's also nice and simple so let's get that in first and keep that issue's patch size small.
Patch when I have a nid.
Comment | File | Size | Author |
---|---|---|---|
#18 | 2166195-htmlpage-18.patch | 18.82 KB | dawehner |
#15 | 2166195-htmlpage-15.patch | 18.23 KB | dawehner |
#4 | htmlpage-2166195.patch | 13.92 KB | dawehner |
#1 | 2166195-split-renderer.patch | 10.64 KB | Crell |
Comments
Comment #1
Crell CreditAttribution: Crell commentedComment #4
dawehnerI really like the cleanup as it makes the two services straightforward at least in the core implementation.
Just some simple cleanup to get it green.
Comment #6
dawehnerComment #8
dawehnerHa, unit tests sometimes should help.
Comment #9
Crell CreditAttribution: Crell commentedOnce again, I post a half-arsed patch and dawehner cleans up my mess. Story of my life...
Thanks, dawehner!
Comment #10
catchIt's really nice to see this code disappearing, but I don't get how we managed to just nuke that in this patch.
Comment #11
Crell CreditAttribution: Crell commentedHrm. I think that's Git's diff weirdness. That code is not going away, it's just moving from DefaultHtmlPageRenderer to DefaultHtmlFragmentRenderer.
Git sees this patch as renaming this class, then removing stuff from it, then adding a new DefaultHtmlPageRenderer.
Comment #12
webchickSince catch looked at this before, and committed the parent patch, shooting over to him.
Comment #13
catch8: wscci-2166195.patch queued for re-testing.
Comment #15
dawehnerRerolled
Comment #16
Crell CreditAttribution: Crell commentedComment #17
alexpottNeeds a reroll
Comment #18
dawehnerRerolled.
Comment #19
Crell CreditAttribution: Crell commentedSigh
Comment #20
catchCommitted/pushed to 8.x, thanks!
Comment #22
Wim LeersRelated: #2327277: [Meta] Page rendering meta discussion.