469832 - In-product mode should be applicable to all articles and search results

Status: VERIFIED FIXED

(support.mozilla.org :: Knowledge Base Software, task)

Description

[Chris Ilias [:cilias]]

We're no longer going to limit the set of articles for in-product help. "?style_mode=inproduct" should be applicable to any article, and even the search results page, for those who search via the product help start page.

Comment 2

[David Tenser [:djst]]

To clarify and confirm, the style_mode=inproduct parameter should be kept
throughout sessions that were started from Firefox (the product). The only way
to "get out of" in-product mode would be to log in.

Comment 3

[Paul Craciunoiu [:paulc]]

Can someone point me to (who has) more information about the inproduct value and what it means?

Comment 4

[Chris Ilias [:cilias]]

Is this what you want: <https://bugzilla.mozilla.org/show_bug.cgi?id=412265>.

Comment 5

[Paul Craciunoiu [:paulc]]

Yes, thanks Chris!

Comment 6

[Paul Craciunoiu [:paulc]]

As I see it, adding a "style_mode=inproduct" to any article URL achieves the desired result. 
To double check, this bug is about having the inproduct mode persist, once enabled, until you log in, right? That is, every article and search result will be viewed as if there is a "style_mode=inproduct" parameter part of the URL.

Comment 7

[Chris Ilias [:cilias]]

Yes. The problem right now is that it gets removed when a user searches.

Comment 8

[Paul Craciunoiu [:paulc]]

r22330 / r22333
* added tiki-searchresults.tpl for mozip
* inproduct mode is set and kept for all pages (including search results), for anonymous users. Logging in gets out of inproduct mode.

Comment 9

[Paul Craciunoiu [:paulc]]

I should mention that I did not set the inproduct mode ONLY if users are viewing the site from Mozilla Firefox, but for any browser. I could change that, as per comment 2. Sorry, I just realized that now.

Comment 10

[Chris Ilias [:cilias]]

I went to <http://support-stage.mozilla.org/en-US/kb/Menu+reference?style_mode=inproduct>, then entered "test" in the search field, and I was not kept in inproduct mode.
Maybe anther problem picking up changes from SVN?

Comment 11

[Paul Craciunoiu [:paulc]]

Yeah, I don't think SVN changes are visible yet.

I should mention that inproduct mode is set regardless of whether the parameter is present in the URL or not, at any time. Is that the way we want it?

Comment 12

[Chris Ilias [:cilias]]

We have a couple of bugs regarding in-product mode in the URL
Bug 422828 - "mod_rewrite style_mode=inproduct to something less ugly"
Bug 463045 - "style_mode=inproduct is part of anchors"

Personally, I would rather fix those and keep it in the URL, than hide it. Especially or testing. But those are justifiably set to SUMO 1.1; so unless the current method is making it hard to fix this bug, I wouldn't change it.

David, do you agree?

Comment 13

[Paul Craciunoiu [:paulc]]

The way it works right now, I set a cookie for anonymous users to have them be inproduct mode at ALL times, on ANY page. Basically, anonymous users only see pages in inproduct mode.

If we want that to only happen if they visit a page using the URL parameter, or only with the Mozilla browser, please say so until tomorrow night. I get the feeling that I didn't do quite what we wanted. I wish my commits were updated on stage so you could test it.

Anyway, if this is not the way we want it, changing it to keep inproduct mode throughout sessions (or for this session) only AFTER it is triggered from the URL is easy.

It seems that Bug 463045 could be extended to all URLs when "style_mode=inproduct" is present in the URL initially.

Comment 14

[Paul Craciunoiu [:paulc]]

support-stage has been fixed now, so you can see what changes have taken place. Please take a look.

Comment 15

[David Tenser [:djst]]

(In reply to comment #13)
> The way it works right now, I set a cookie for anonymous users to have them be
> inproduct mode at ALL times, on ANY page. Basically, anonymous users only see
> pages in inproduct mode.
> 
> If we want that to only happen if they visit a page using the URL parameter, or
> only with the Mozilla browser, please say so until tomorrow night. 

This is not the wanted behavior. Only when the parameter is used, the style should be used.

Otherwise, people coming from www.mozilla.com and clicking on Support will get a different header once they click on an article. 

We only want to distinguish between visitors being directed to SUMO from Firefox (the product) and visitors visiting the website the normal way.

Comment 16

[Paul Craciunoiu [:paulc]]

Changed:
* inproduct mode only triggered by URL parameter. A cookie is set and kept throughout sessions.
* can go out of product mode by logging in, OR adding "style_mode=outproduct" to URL, which deletes the cookie. Going back to inproduct can only happen through URL parameter.

Try that.

Comment 17

[Chris Ilias [:cilias]]

I redid the steps in comment 10. The search results page displayed in inproduct mode. I clicked on the first result, which was <http://support-stage.mozilla.org/en-US/kb/Using+the+Flash+plugin+with+Firefox>, and that page displayed in inproduct mode.

I quit Firefox then started it again, then went to <http://support-stage.mozilla.org/en-US/kb/Using+the+Flash+plugin+with+Firefox>, but it was still in inproduct mode. I had a look at the cookie, and it expires "Fri, Mar 20, 2009 11:01:49 PM" which is roughly 25 hours after it was stored.

Also, will this affect caching? Will people who don't have the cookie get a cached inproduct mode version of the URL?

Comment 18

[Paul Craciunoiu [:paulc]]

Going to http://support-stage.mozilla.org/en-US/kb/Using+the+Flash+plugin+with+Firefox?style_mode=outproduct
seems to work.

It appears that caching keeps the inproduct version, if that was visited first.

Comment 19

[David Tenser [:djst]]

Sorry, is it a problem to solve this the "right way," meaning the inproduct mode is only used when the parameter style_mode=inproduct is used? The solution implemented now messes with caching and is not the intended design.

Comment 20

[Paul Craciunoiu [:paulc]]

r22425 / r22426
This will keep the URL parameter at all times while the cookie is set. It won't mess with the caching, but the anchor part of the URL will be lost, since that does not get sent to PHP. It's a minor inconvenience, though.

To keep the anchor we would have to send it to PHP through JS (more complicated), or modify the Smarty template files for all pages where we want the style_mode appended (simpler, but significant changes need to be made). I could do the second, but I'm not sure how many template files there would be, and I probably won't be able to finish today. Also, all the links within articles that are hardcoded would need to be processed somehow...

However, the problem mentioned in comment 7 will be fixed by the changes I just made.
Maybe Eric has some quick ideas. Eric?

Comment 21

[Chris Ilias [:cilias]]

This looks fixed to me. The anchor issue is bug 463045.

Comment 22

[Chris Ilias [:cilias]]

I spoke too soon. I cleared my support-stage.mozilla.org cookie, restarted Firefox, went to <http://support-stage.mozilla.org/en-US/kb/Menu+reference?style_mode=inproduct>, the entered "test" in the search bar, and search results page was not in inproduct mode.

Comment 23

[Paul Craciunoiu [:paulc]]

Thanks for testing this Chris! I moved the redirect to style_mode=inproduct and setting the cookie before caching. That way, caching still happens, but the redirect and cookie setting happens before that. Caching would skip the cookie being set... so that's why it didn't work.

Good catch Chris!
r22445 / r22446

Comment 24

[E. Cooper [:ecooper]]

r22445 causes a regression for loading the correct style sheets.

$cat_type and $cat_objid can't be removed from init or things like mozms.css and mozad.css won't be loaded.

http://support-stage.mozilla.org/en-US/kb/

Comment 25

[E. Cooper [:ecooper]]

(In reply to comment #20)
> Maybe Eric has some quick ideas. Eric?

FWIW, I plan on looking at this again during lunch.

Comment 26

[E. Cooper [:ecooper]]

(In reply to comment #24)
> r22445 causes a regression for loading the correct style sheets.
> 
> $cat_type and $cat_objid can't be removed from init or things like mozms.css
> and mozad.css won't be loaded.
> 
> http://support-stage.mozilla.org/en-US/kb/

$cat_type and $catobjid aren't the problem. I mistakenly looked a revision thinking it was pre-bug, but it was just a previous patch of this.

In any case, regression still exists.

Comment 27

[Paul Craciunoiu [:paulc]]

Try this, r22465 (r22466 revised).
Committed only on stage for testing this time.
I moved the precache stuff into a separate file, that's now being called at the end of tiki-setup. tiki-tc.php and tiki-index.php remain as before this bug fixes.

Please test.

Comment 28

[E. Cooper [:ecooper]]

Attachment: Alternative method to preserve inproduct mode

This is my take on the bug. If all we want is to preserve the inproduct mode across searches and apply the mozip.css style sheet over all categories, something like this would work. I feel changing two lines to modify current behavior is better than adding all new behavior.

Oddly, I noticed that only the first five search results of each page get the inproduct mode param appended. That's a separate bug, and is probably WONTFIX if we are to be using the new search soon.

Anyway, my two cents...

If this isn't a 0.9 bug its changes should be removed from the production branch before the release is tagged. Otherwise, the new behavior will be present on production.

Comment 29

[Paul Craciunoiu [:paulc]]

I say remove changes from production also. This bug isn't fixed yet.

Comment 30

[Paul Craciunoiu [:paulc]]

Oh yeah, the bug was meant for 0.9 so we will see the changes on production, I think... What can be done about that?

Comment 31

[Laura Thomson :laura]

Paul, can  you please revert these changes from the prod branch before we tag 0.9?

Comment 32

[Paul Craciunoiu [:paulc]]

I'll be able to do that tonight, in the evening. Is it too late? I'm having an extra busy day.

Comment 33

[E. Cooper [:ecooper]]

If you find you don't have time tonight for whatever reason, you can ping me and I'll do it with my other stuff.

Comment 34

[Paul Craciunoiu [:paulc]]

Done, rolled back. r22586 / r22587.

I like Eric's suggestion in comment 28, it's more specific to this bug.
It is worth mentioning that having a session cookie would allow for a more site-wide way of preserving inproduct mode.

So, to make this quick, we could go with Eric's solution. If we may need to expand inproduct mode over more files, it's probably better to test a bit more and adopt the solution I posted in comment 27. I think either is fine, at this point.

Comment 35

[David Tenser [:djst]]

Looks like a fix is almost ready, but Firefox 3.1 is no longer a Q1 target, so moving this out of SUMO 1.0. Feel free to submit a patch anyway.

Comment 36

[Paul Craciunoiu [:paulc]]

Eric, has stuff changed that would allow for a different/easier fix on this bug?

Comment 37

[Paul Craciunoiu [:paulc]]

Anything on this, Eric?

Comment 38

[Chris Ilias [:cilias]]

I came across this when testing new search, and realized that we forgot about this bug (at least I did). I don't know if it's too late to target for 1.1.

Comment 39

[Paul Craciunoiu [:paulc]]

I wouldn't want to rush and miss something tonight. Two things need to be done, at least:
1. Search box(es) on all pages should preserve style_mode when set in the URL
2. Search results should do the same, on the search page itself.

Those are the only two transitions from and to search, right?

Comment 40

[Laura Thomson :laura]

Yes, too late for 1.1, sorry.  (I don't want to put any more hurdles in its path.)  But we will definitely sort it out for 1.2.

Comment 41

[Paul Craciunoiu [:paulc]]

Attachment: Preserve inproduct mode in newsearch

This is inspired by Eric's patch (attachment 363633 [details] [diff] [review]).
It updates the two search bars for inproduct mode, and the URLs in the search results to all preserve inproduct mode.

The part that needs most review (and QA) is the url filter update. Change:
- previously, it only parsed URLs that started with /<locale>/kb/...style_mode=...
- now it parses all urls that have /kb/ and [?&]style_mode in them (i.e. do not necessarily start with /<locale>/kb)

Comment 42

[Laura Thomson :laura]

I have started review on this, but I'm not sure about the url filter.  What are the cases you are trying to address here?

Comment 43

[Paul Craciunoiu [:paulc]]

(In reply to comment #42)
> I have started review on this, but I'm not sure about the url filter.  What are
> the cases you are trying to address here?
1. It doesn't currently work. Remember that I haven't checked anything in yet, and see these two:
http://support-stage.mozilla.org/tiki-newsearch.php?where=f&locale=en-US&q=browser&sa=&style_mode=inproduct
(check the search results URL: forum threads preserve parameter)
http://support-stage.mozilla.org/tiki-newsearch.php?where=d&locale=en-US&q=browser&sa=&style_mode=inproduct
(check the search results URL: KB articles do NOT! preserve parameter)
2. It will cover more general cases (sometimes maybe people will mistakenly include external, full URL to "http://s.m.com/locale/kb/...)

Comment 44

[Paul Craciunoiu [:paulc]]

I should add that the reason for 1. above is because URLs start with e.g. en/kb, instead of /en/kb. So a fix that would change less would be to just make the first slash optional. Laura: want me to try that?

Comment 45

[E. Cooper [:ecooper]]

Comment on attachment 379807 [details] [diff] [review]
Preserve inproduct mode in newsearch

The change to the output filter will actually break kb links in articles (...been there).

Instead of changing webroot/lib/smarty_tiki/outputfilter.urlt.php:294, we should just change how the link is formed in webroot/templates/tiki-newsearch.tpl

Comment 46

[E. Cooper [:ecooper]]

Attachment: changing tiki-newsearch.tpl instead of smarty filter

Excerpt of alternative route mentioned in comment 45.

Comment 47

[Paul Craciunoiu [:paulc]]

Attachment: Preserve inproduct mode in newsearch, v2

Okay, merged into the entire patch.

Comment 48

[Laura Thomson :laura]

Comment on attachment 380857 [details] [diff] [review]
Preserve inproduct mode in newsearch, v2

I think this is ok but it gives me the fear.

Comment 49

[Paul Craciunoiu [:paulc]]

r27982 / r27983
Please extensively QA this :)

Comment 50

[Stephen Donner [:stephend] Not actively reading bugmail]

When I follow the steps in comment 43, namely, by clicking on http://support-stage.mozilla.org/tiki-newsearch.php?where=f&locale=en-US&q=browser&sa=&style_mode=inproduct and then its first result, http://support-stage.mozilla.org/tiki-view_forum_thread.php?style_mode=inproduct&locale=en-US&comments_parentId=324234&forumId=1, I still see the full Mozilla.com header.

I've read all the comments here, but am confused; while I can verify that we're getting style_mode=inproduct appended to/maintained on URLs, the right style sheet isn't being applied -- is that a separate bug?

Comment 51

[Paul Craciunoiu [:paulc]]

(In reply to comment #50)
> getting style_mode=inproduct appended to/maintained on URLs, the right style
> sheet isn't being applied -- is that a separate bug?
I think so. Currently, on SUMO, a thread inproduct page looks the same:
http://support.mozilla.com/tiki-view_forum_thread.php?locale=en-US&comments_parentId=260574&forumId=1&style_mode=inproduct
I'm not sure if it was ever displayed with the inproduct header, but it's a separate bug.

Comment 52

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #51)
> (In reply to comment #50)
> > getting style_mode=inproduct appended to/maintained on URLs, the right style
> > sheet isn't being applied -- is that a separate bug?
> I think so. Currently, on SUMO, a thread inproduct page looks the same:
> http://support.mozilla.com/tiki-view_forum_thread.php?locale=en-US&comments_parentId=260574&forumId=1&style_mode=inproduct
> I'm not sure if it was ever displayed with the inproduct header, but it's a
> separate bug.

Filed bug 499699.

Comment 53

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED; saw the &style_mode=inproduct parameter persist between searches and KB articles, as well as disappear when logging in (as it's supposed to).

I'll continue to ad-hoc test from now on -- would could potentially regress with this patch?

Comment 54

[Paul Craciunoiu [:paulc]]

Comment on attachment 380857 [details] [diff] [review]
Preserve inproduct mode in newsearch, v2

Just going through my r? list