Closed Bug 521706 Opened 13 years ago Closed 11 years ago

Add last modified date to footer where the current 'Page History' link is

Categories

(www.mozilla.org :: General, enhancement, P2)

enhancement

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: arkanoid7294, Assigned: GPHemsley)

References

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3 (.NET CLR 3.5.30729)

Add last modified date to footer where the current 'Page History' link is.


Reproducible: Always
Priority: -- → P2
Assignee: nobody → reed
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: reed → paul
Question: would we like the last modified date to be a link to the page history? Or do away with that link altogether?
I think it makes sense to keep it as a link to the same place 'Page history' is pointing now.
I used fstat to get the mtime for the file including footer.inc.php. This was faster than doing any SVN magic and easier than Keywords which would make it difficult to get any information for the file including footer.inc.php and not for footer.inc.php itself.
Comment on attachment 446987 [details] [diff] [review]
Modifies includes/footer.inc.php and includes/MozillaOrg/SvnHistoryLink.php to use Last Modified date of page as link text.

Reed, can you review and check in if this looks ready to go?
Attachment #446987 - Flags: review?(reed)
Comment on attachment 446987 [details] [diff] [review]
Modifies includes/footer.inc.php and includes/MozillaOrg/SvnHistoryLink.php to use Last Modified date of page as link text.

>+    public static function getLastModifiedDate($file, $fmt = '%A %B %e %Y')
>+    {
>+        if (!is_readable($file)) {
>+    	    return;
>+        }
>+
>+        $fp = @fopen($file, 'r');
>+      
>+        if ($fp === FALSE) {
>+	    return;
>+        }
>+
>+        $fstat = fstat($fp);
>+        fclose($fp);
>+
>+        if (isset($fstat['mtime'])) {
>+	    return strftime($fmt, $fstat['mtime']);
>+        }
>+    }

Is there a reason why all this file pointer stuff is used, instead of just filemtime() and date()?
Nope, good catch, filemtime() and date() would be better. Updated would look something like this then:

public static function getLastModifiedDate($file, $fmt = 'l F j Y')
{
    if (is_readable($file)) {
        return date($fmt, filemtime($file));
    }
}
(In reply to comment #6)
> Nope, good catch, filemtime() and date() would be better. Updated would look
> something like this then:
> 
> public static function getLastModifiedDate($file, $fmt = 'l F j Y')
> {
>     if (is_readable($file)) {
>         return date($fmt, filemtime($file));
>     }
> }

Yeah, that's much better.

Also, what about the appendPath() function? The comments in the config file say that $config['file_root'] is not supposed to have a trailing slash. Is there a reason you don't just do this?:
$path     = $config['file_root'] . '/' . $filename;

As for the dirname(dirname(__FILE__)) business at the top of the file, is that standard for the Mozilla.org codebase? What's wrong with a simple '../..'? (I'm asking because I'm not familiar with the Mozilla.org codebase.)
Patch applied to Kildare branch in r76032.
Comment on attachment 446987 [details] [diff] [review]
Modifies includes/footer.inc.php and includes/MozillaOrg/SvnHistoryLink.php to use Last Modified date of page as link text.

r- based on Gordon's comments. I'll look at it again once all those issues and questions are answered.
Attachment #446987 - Flags: review?(reed) → review-
(In reply to comment #8)
> Patch applied to Kildare branch in r76032.

Please either remove the patch or fix the existing issues.
(In reply to comment #10)
> Please either remove the patch or fix the existing issues.

Reverted for now in r76038.
If this requires more work then we can leave it out of the home page refresh, but I want us to find a way to move forward with this.  I don't think Paul has time to drive this, so Reed or Gordon would either of you be interested in taking this?

BTW, it seems a little wordy the way it is now.  I'd recommend changing:

"This page was last modified on Tuesday October 12 2010"

to 

"Last modified on October 12, 2010"

* The first few words are removed
* The day is removed
* A comma is added after the date
Attached patch Patch incorporating comments (obsolete) — Splinter Review
I believe this patch incorporates all the feedback comments.
Assignee: paul → gphemsley
Attachment #446987 - Attachment is obsolete: true
Attachment #484746 - Flags: review?
Attachment #484746 - Flags: review? → review?(reed)
This is r82448 as applied to staging. It fixes bitrot from the past couple of months. It assumes (and somewhat supersedes) bug 619842, which was caused by the fix to bug 619263.

It also includes some miscellaneous whitespace changes (trailing spaces, detabbing, proper indentation) and removes a redundant empty "else" left over from debug code removal in r80480 (from the patch for bug 521704, originally committed in r80454).

It appears to be working fine on stage. I'll need someone (e.g. Reed) to merge it to trunk.
Attachment #484746 - Attachment is obsolete: true
Attachment #511650 - Flags: review?(reed)
Attachment #484746 - Flags: review?(reed)
Assignee: gphemsley → reed
Status: NEW → ASSIGNED
Severity: trivial → enhancement
Assignee: reed → gphemsley
Depends on: 619842
Sending        includes/MozillaOrg/SvnHistoryLink.php
Sending        includes/footer.inc.php
Transmitting file data ..
Committed revision 91176.

Gerv
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Looks good; marking as VERIFIED.
Status: RESOLVED → VERIFIED
Attachment #511650 - Flags: review?(reed)
Component: www.mozilla.org → General
Product: Websites → www.mozilla.org
You need to log in before you can comment on or make changes to this bug.