Search Results Page - Rendered with Incorrect CSS Cascade after Upgrading from 7.x-1.2 to 7.x-1.3 [#1899306]

After upgrading search_by_page on my test site, I noticed that the search results were not rendering correctly. The problem was related to the order that Drupal added the CSS files. Drupal would return the correct order on any other page.

Comment	File	Size	Author
#14	1899306-overhaul.patch	9.28 KB	jhodgdon
#14
#7	Diff of the rendered CSS from the front page to the search results page.	2.51 KB	sjbassett
#6	1899306-reset-css-6.patch	1.51 KB	jhodgdon
#6
#4	css-issue.png	218.04 KB	sjbassett
#4	1899306-CSS-Rendering-Order-Issue.patch	530 bytes	sjbassett
#4
#2	example.diff	3.26 KB	sjbassett
#2
	Screen Shot of Rendered Search Results After Upgrade to 7.x-1.3	152.33 KB	sjbassett
	Screen Shot of Rendered Search Results Before Upgrade to 7.x-1.3	106.39 KB	sjbassett

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 25 January 2013 at 22:02

Status:

Active

» Postponed (maintainer needs more info)

Can you tell me what changed in the order that Drupal added the CSS files?

Comment #2

sjbassett CreditAttribution: sjbassett commented 28 January 2013 at 14:45

File	Size
example.diff	3.26 KB

Sorry, should have included that before. I simple created a diff of the section of the markup declaring stylesheets. The 'A' order is the desired order for the drupal page to render. The 'B' order is what is causing the issue in the CSS on the search results page. You can see that the order has changed quite a bit. This is only after upgrading the module.

Comment #3

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 28 January 2013 at 15:58

Status:

Postponed (maintainer needs more info)

» Active

Interesting... A bit of background:

When Search by Page gathers search results, if it is asked to do an excerpt in order to highlight search terms, it has to fully render each page that would be part of the search result, in order to make the excerpt (that is how Search by Page is able to ensure that it is indexing/searching/highlighting all of the text in the content region of each page, which is its central feature).

The process of rendering pages means that for each page that is excerpted, the Drupal theme, block, and menu systems get initialized. Then, before actually rendering the search results page itself, all of this information has to be cleared out so that the search results page itself will render properly.

Between 7.x-1.2 and 7.x-1.3, some bugs were fixed in this process, so that now more information is being cleared out between the rendering of pages to get excerpts and the rendering of the search results page itself.

That's the background... For your issue, I think there are three possibilities:

a) The previous order of CSS that you had could be actually due to a bug in 7.x-1.2 where the CSS was not being reset between excerpts and results page rendering, and now that this bug is fixed, your page doesn't look right with the "correct" order of CSS.

b) There is a new bug in 7.x-1.3 regarding CSS resetting that wasn't present in 7.x-1.2.

c) There may be a bug in both 7.x-1.2 and 7.x-1.3, where CSS is somewhat reset in 7.x-1.2 and somewhat reset in 7.x-1.3 and they are working differently, but both wrong.

I'll need to investigate to see if I can figure out which possibility is actually happening, and how to fix it.

My guess is that in the function search_by_page_page_content(), which does the rendering of pages for excerpts, I need to add lines at the end that will reset both CSS and JS, which are not currently being reset.

That would mean adding two lines just before the end return $content; line:

drupal_static_reset('drupal_add_css');
drupal_static_reset('drupal_add_js');

If you would like to try that on your site and let me know what you find out, that would be helpful! Although if both 7.x-1.2 and 7.x-1.3 had CSS-related bugs, it may not give you what you want for CSS output...

Comment #4

sjbassett CreditAttribution: sjbassett commented 29 January 2013 at 15:21

File	Size
1899306-CSS-Rendering-Order-Issue.patch	530 bytes

css-issue.png	218.04 KB

Thank you @jhodgdon for your detailed response. I really appreciate your insight. So, the suggested change did resolve the issue of the correct CSS order on the page, see the code snippet below. However if you see the screen shot the hidden items are now all being shown.

Screen shot of CSS issue in the search by page module.

@import url("http://library/sites/all/themes/nulib/css/html-reset.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/wireframes.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/page-backgrounds.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/tabs.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/pages.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/blocks.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/navigation.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/views-styles.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/nodes.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/comments.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/forms.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/fields.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/corrections.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/style.css?mhe7q9");
@import url("http://library/sites/all/themes/nulib/css/nulib.css?mhe7q9");

Comment #5

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 29 January 2013 at 15:28

OK, thanks for testing! Probably the fix needs to be more complicated. As you'll notice, at the top of that function, there's some code that saves the state of a few things temporarily, which are then restored at the bottom. Probably the CSS and JS need to be done like that rather than completely reset to nothing as in the simple fix I had proposed.

I'll work on another patch that does that.

Comment #6

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 29 January 2013 at 16:02

Status:

Active

» Needs review

File	Size
1899306-reset-css-6.patch	1.51 KB

Could you test this patch?

Comment #7

sjbassett CreditAttribution: sjbassett commented 8 February 2013 at 21:54

File	Size
Diff of the rendered CSS from the front page to the search results page.	2.51 KB

@jhodgdon, the issue still is not ressolved on my test site.

Comment #8

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 8 February 2013 at 22:13

Is that with the patch in #6 applied?

The thing is, the previous behavior you were seeing may be what you built your theme's CSS around, but it may not have been correct... I think with the patch in #6 applied, the CSS should be correct and stable now, and it should be the same no matter what you search for. Without that patch, I imagine that some search terms could give you different CSS output.

And... I am not of course familiar with your theme. Which CSS files do you think should be output that aren't being output on the search results page, or which ones do you think are in the wrong order?

Comment #9

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 9 February 2013 at 01:09

Actually, I'd be interested in knowing if there is any difference in the CSS you see for:

a) Searching for something that gives you a result that's a node page, if you have Search by Page Nodes turned on.
b) Searching for something that returns a result that's a Views page or something else you put in via Search by Page Paths, if you have that turned on.
c) Searching for something that gives you a User page if you have Search by Page Users turned on.
d) Searching for something like xasdfsadfs that has no results.

I think that they **should** all result in the same CSS getting on the page, ideally. I'd be interested on your site if they do for 7.x-1.2, for 7.x-1.3, and for 7.x-1.3 with the patch in #6.

This is difficult for me to test, because on my test site, I don't think there is any real difference between the CSS from page to page anyway. I'm assuming that on your site, there are differences between the CSS on different pages, and that some of the CSS from the nodes, user pages, and Paths pages is bleeding into your search results page. I'm hoping that the patch in #6 has stopped that from happening, but you may have already themed your site based on that happening. Does that make sense?

Comment #10

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 11 February 2013 at 22:45

Status:

Needs review

» Postponed (maintainer needs more info)

Comment #11

guictx CreditAttribution: guictx commented 14 April 2013 at 10:53

I can confirm this issue using 7.x-1.3 and 7.x-1.x-dev.

When searching for users and nodes, pages with results have the theme css order reversed. Pages with no results have the correct order.

Patch in #4 against latest dev solves the problem.
Patch in #6 against latest dev doesn't solve the problem.

FYI, in my theme I am excluding most of the css coming from drupal and modules.

Comment #12

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 15 April 2013 at 14:40

Hm. I don't think the patch in #4 is going to work for all users, because I think some CSS and JS will be added before Search by Page gets around to starting its rendering of the search results, and patch #4 would reset all of the CSS to nothing after rendering. It seems like it might lose information.

I'll have to check on the patch in #6 again and see if I can make it better.

Comment #13

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 13 May 2013 at 20:55

Status:

Postponed (maintainer needs more info)

» Fixed

This and various other bugs have cropped up from time to time in Search by Page... so I decided to do somewhat of an overhaul of how Search by Page generates the search results page -- a change in philosophy in order to take care of this and all the other related issues.

Previously, when generating search results, Search by Page would go through each hit in the search results, render the page again, and use that to generate the excerpt shown on the results page. This has caused all kinds of problems such as this issue, active trail problems, and many more, becuase it was kind of a hack of Drupal's page rendering process.

So, now things are being done differently. I've just made a commit that makes the following changes (patch attached for reference, and it will show up in the source control repo immediately, in the 7.x-1.x-dev version within about 12 hours, and in the next full release whenever that is):
- When indexing a page, Search by Page now stores the page content that was rendered at that time.
- When building search results, the Search by Page Nodes, Users, and Paths modules, instead of rendering the page again (which was causing all those problems), use the stored page content to make their search excerpt.

If you install this update, you will need to:
a) run update.php to get the new database field for storing this new information
b) run cron until Search by Page is fully reindexed. Until you do this, your search results will show up with just the page title and no excerpts.
c) if you are using any Search by Page sub-modules that are not distributed with Search by Page, they will need to be updated so that they do not try to render pages when search results are being presented.

I am marking this issue "fixed"... hopefully it really is fixed this time! Any testing you can do would be greatly appreciated -- report back!