505526 - "Go Back" link should be disabled when the page is directly linked

Status: RESOLVED INCOMPLETE

(support.mozilla.org :: General, defect)

Description

[krupa raj[:krupa]]

steps to reproduce:
1.Set a watch on any of the articles.
2.Ensure that the article is modified.
3.In the email notification,right click on the link under -"If you don't want to
receive these notifications follow this link:" message and open it in a new window

expected results:
"go back" link in the page is disabled

actual result:
"go back" is active and nothing happens on clicking on it

Comment 1

[Paul Craciunoiu [:paulc]]

This affects a broad-range of pages so we should discuss what pages of this type should look like before we decide what the fix should involve.

Comment 2

[Paul Craciunoiu [:paulc]]

I've been thinking about this. Implementation-wise, here are a few suggestions on a better approach:
* Show "go back" if the tab has history
* Show a URL to _somewhere_ otherwise

Comment 3

[Paul Craciunoiu [:paulc]]

Attachment: patch, v1

Comment 4

[Laura Thomson :laura]

Comment on attachment 391424 [details] [diff] [review]
patch, v1

The referer and js history are not strictly interchangable...test it on the client side.

Comment 5

[Paul Craciunoiu [:paulc]]

(In reply to comment #4)
> (From update of attachment 391424 [details] [diff] [review])
> The referer and js history are not strictly interchangable...test it on the
> client side.
I know. I misspoke in comment 2. Do we want the patch to rely on JS? I think doing it with PHP makes things much simpler.

Comment 6

[Laura Thomson :laura]

The advantage of using JS is if they come from a non SUMO website they can go back there.

Comment 7

[Paul Craciunoiu [:paulc]]

Attachment: patch, v1

This uses javascript history.

Comment 8

[James Socol [:jsocol, :james]]

IE's history.length starts at 0, not 1, hiding the links in IE if it's the second page the user visits.

Maybe we can put the links in by default with some extra class, then remove them onload via jQuery? That would also eliminate document.write().

Comment 9

[Paul Craciunoiu [:paulc]]

Sounds good James, want a stab at it?

Comment 10

[James Socol [:jsocol, :james]]

Attachment: patch,v3

Uses jQuery to strip out the links instead of using document.write() to add them, and fixes the behavior in IE6/7.

Comment 11

[Paul Craciunoiu [:paulc]]

Comment on attachment 394138 [details] [diff] [review]
patch,v3

I like the implementation, here's should be changed:

* instead of adding the JS inside the page, use $headerlib->add_js + $headerlib->add_js file (see webroot/lib/headerlib.php). This will all be output in webroot/templates/styles/mozkb/footer.tpl

* We are also using minify which already loads jquery.min.js - see webroot/minify.php:27

Comment 12

[Paul Craciunoiu [:paulc]]

(In reply to comment #11)
sorry, I meant ->add_js_file

Comment 13

[James Socol [:jsocol, :james]]

Attachment: patch, v4

Moved JavaScript to $headerlib->add_js(), stopped including jQuery, and moved the .go-back class to containing elements when applicable.

Comment 14

[Paul Craciunoiu [:paulc]]

Comment on attachment 394329 [details] [diff] [review]
patch, v4

What's {ldelim} and {rdelim}? I had to remove those to get the link to hide. That worked, but there's too much white space left over.

Let's just leave this bug till we do more p12n stuff.

Comment 15

[James Socol [:jsocol, :james]]

{ldelim} and {rdelim} are how you get braces into code parsed by Smarty. I had to add them because it was parsing the JavaScript as Smarty code:

  function(){if

Taking them out caused fatal PHP errors on mine. But we can definitely hold off, it's hardly crucial.

Comment 16

[Paul Craciunoiu [:paulc]]

Some minor fixes on this. James, if you have time after we're done with p12n bugs, you can patch this up too. But no worries :)