Closed
Bug 414509
Opened 17 years ago
Closed 14 years ago
offer View All (non obsolete) attachments
Categories
(Bugzilla :: Attachments & Requests, enhancement)
Tracking
()
RESOLVED
FIXED
Bugzilla 4.2
People
(Reporter: timeless, Assigned: guy.pyrzak)
References
()
Details
Attachments
(1 file, 2 obsolete files)
3.03 KB,
patch
|
LpSolit
:
review+
|
Details | Diff | Splinter Review |
bug 413961 has too many attachments, i want to: 1. click "hide obsolete" 2. click "view all" *see* only the non obsolete attachments. note: i don't want to break current urls, i just want a new feature &action=viewnonobsolete
Comment 1•17 years ago
|
||
Yeah, that sounds like the right implementation and a sensible thing to do.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Mac OS X → All
Hardware: Macintosh → All
Comment 2•17 years ago
|
||
Oh, except it should be &hide_obsolete=1, not a new whole action.
Assignee | ||
Comment 3•17 years ago
|
||
Yoink! (the sound of taking a bug)
Assignee: attach-and-request → guy.pyrzak
Assignee | ||
Comment 4•14 years ago
|
||
I'm attaching this for now but I think I need to add something to the page to indicate that obsolete attachments have been hidden.
Updated•14 years ago
|
Attachment #430979 -
Attachment is patch: true
Attachment #430979 -
Attachment mime type: application/octet-stream → text/plain
Assignee | ||
Comment 5•14 years ago
|
||
Attachment #430979 -
Attachment is obsolete: true
Attachment #430982 -
Flags: review?(LpSolit)
Comment 6•14 years ago
|
||
Comment on attachment 430982 [details] [diff] [review] V2 >=== modified file 'skins/standard/attachment.css' >+#hidden_obsolete_message{ Missing whitespace before {. >=== modified file 'template/en/default/attachment/list.html.tmpl' >+ [% bugid %]&action=viewall >+ [%- "&hide_obsolete=1" IF obsolete_attachments %]">View All</a> & -> & (must be escaped). >=== modified file 'template/en/default/attachment/show-multiple.html.tmpl' >+ Obsolete attachments are hidden. To view all attachments (including obsolete) <a href="attachment.cgi?bugid=[% bug.id %]&action=viewall">click here</a>. [% bug.id %] must be HTML filtered: # Failed test '(en/default) template/en/default/attachment/show-multiple.html.tmpl has unfiltered directives: # 38: bug.id # --ERROR' # at t/008filter.t line 132. Otherwise looks good and works great!
Attachment #430982 -
Flags: review?(LpSolit) → review-
Updated•14 years ago
|
Target Milestone: --- → Bugzilla 3.8
Updated•14 years ago
|
Target Milestone: Bugzilla 4.0 → ---
Comment 7•14 years ago
|
||
Thinking about it again, I think that "View All" should only display non-obsolete attachments, always.
Comment 8•14 years ago
|
||
(In reply to comment #7) > Thinking about it again, I think that "View All" should only display > non-obsolete attachments, always. Or perhaps just whatever is currently being displayed in the table, no?
Comment 9•14 years ago
|
||
(In reply to comment #8) > Or perhaps just whatever is currently being displayed in the table, no? Sure, this would work too.
Assignee | ||
Comment 10•14 years ago
|
||
Attachment #430982 -
Attachment is obsolete: true
Attachment #474179 -
Flags: review?(LpSolit)
Comment 11•14 years ago
|
||
Comment on attachment 474179 [details] [diff] [review] v3 >=== modified file 'attachment.cgi' >+ if ( $cgi->param('hide_obsolete') ) { Nit: no need for the extra whitespaces in parens. >+ $attachments = [ grep { ! $_->isobsolete } @$attachments ]; Please write: @$attachments = grep { !$_->isobsolete } @$attachments; === modified file 'template/en/default/attachment/list.html.tmpl' >+ <a id="view_all" href="attachment.cgi?bugid= >+ [% bugid %]&action=viewall Nit: please write [%- bugid %] instead, to make sure no newline is inserted in the URL (those are a pain for Selenium). >=== modified file 'template/en/default/attachment/show-multiple.html.tmpl' >+ <div id="hidden_obsolete_message"> >+ Obsolete attachments are hidden. To view all attachments (including obsolete) >+ <a href="attachment.cgi?bugid=[% bug.id FILTER html%]&action=viewall">click here</a>. >+ </div> Please fix the indentation, and add a whitespace before %]. I like this patch. :) r=LpSolit with my comments fixed.
Attachment #474179 -
Flags: review?(LpSolit) → review+
Updated•14 years ago
|
Flags: approval+
Target Milestone: --- → Bugzilla 4.2
Assignee | ||
Comment 12•14 years ago
|
||
Committing to: bzr+ssh://guy.pyrzak%40gmail.com@bzr.mozilla.org/bugzilla/trunk/ modified attachment.cgi modified skins/standard/attachment.css modified template/en/default/attachment/list.html.tmpl modified template/en/default/attachment/show-multiple.html.tmpl Committed revision 7511.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•