If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

attachment.cgi should not be editable when the user is not logged in

RESOLVED FIXED in Bugzilla 3.6

Status

()

Bugzilla
Attachments & Requests
P2
enhancement
RESOLVED FIXED
11 years ago
7 years ago

People

(Reporter: timeless, Assigned: Frédéric Buclin)

Tracking

2.23.3
Bugzilla 3.6
Dependency tree / graph
Bug Flags:
approval +

Details

(URL)

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

11 years ago
precondition: log out! <https://bugzilla.mozilla.org/relogin.cgi>

if show_bug.cgi is readonly when i'm not logged in
why is attachment.cgi editable when i'm not logged in?

lpsolit says: because attachment.cgi doesn't use Bug::check_can_change_field

Updated

11 years ago
Severity: normal → enhancement
OS: Windows XP → All
Hardware: PC → All
Summary: why is attachment.cgi editable when i'm not logged in? → attachment.cgi should not be editable when the user is not logged in
(Reporter)

Updated

10 years ago
Keywords: ue

Comment 1

10 years ago
not sure why this is a UE thing, seems like a normal bug.
Keywords: ue
(Assignee)

Updated

9 years ago
Priority: -- → P2
Target Milestone: --- → Bugzilla 4.0
(Assignee)

Comment 2

9 years ago
Keep bug 219021 in mind when fixing this bug.
(Assignee)

Updated

9 years ago
Blocks: 476603
(Assignee)

Updated

9 years ago
Depends on: 477662
(Assignee)

Updated

9 years ago
No longer blocks: 476603
(Assignee)

Comment 3

8 years ago
Created attachment 403300 [details] [diff] [review]
patch, v0.9

In this patch, you will notice that I added the creator of the attachment, i.e. it contains what I want to implement in bug 442258. So if you review this patch first, there is no need to review bug 442258 as it will be fixed by this bug.
Assignee: attach-and-request → LpSolit
Status: NEW → ASSIGNED
Attachment #403300 - Flags: review?(guy.pyrzak)
(Assignee)

Updated

8 years ago
Target Milestone: Bugzilla 4.0 → Bugzilla 3.6
(Assignee)

Comment 4

8 years ago
Created attachment 403302 [details]
screenshot for logged out or unprivileged users

Here, you have a screenshot of how the Details page appears to logged out users and to users with not enough privs to edit attachment attributes. The UI is unchanged if you have enough privs to edit attachment attributes.

The comment box is always editable as everybody can comment.

Comment 5

8 years ago
everyone can comment on an attachment when not logged in but the same isn't expected of bugs? I didn't think we allowed anonymous comments.

Updated

8 years ago
Attachment #403300 - Flags: review?(guy.pyrzak) → review-

Comment 6

8 years ago
Comment on attachment 403300 [details] [diff] [review]
patch, v0.9

1. Don't use <br/> for formatting use divs, they can be styled.
2. Add classes to the divs so that someone could style them or at least add it to the container.
3. it looks like you are only changing the type in most of these if else statements. You should put probably just use one else if set an "input_type" variable and use that over and over. It'll make cleaner code IMO.
4. Shouldn't Creator be a term, just a question not a requirement.
5.
(Assignee)

Comment 7

8 years ago
(In reply to comment #5)
> everyone can comment on an attachment when not logged in but the same isn't
> expected of bugs? I didn't think we allowed anonymous comments.

Anonymous comments are not allowed. When you submit it, you will have to log in.

(In reply to comment #6)
> 3. it looks like you are only changing the type in most of these if else
> statements. You should put probably just use one else if set an "input_type"
> variable and use that over and over. It'll make cleaner code IMO.

This won't work. Not only the type changes, but you also have to put the data plain text when not allowed to edit the attachment. So it makes the code harder to read IMO.

> 4. Shouldn't Creator be a term, just a question not a requirement.

No, I don't think that's useful.
(Assignee)

Comment 8

8 years ago
Created attachment 403559 [details] [diff] [review]
patch, v2

No longer use <br> nor <b> nor <small>. Everything has been moved into CSS. Tested successfully on IE 6, IE 8, Firefox, Safari, Opera, Google Chrome and Konqueror.
Attachment #403300 - Attachment is obsolete: true
Attachment #403302 - Attachment is obsolete: true
Attachment #403559 - Flags: review?(guy.pyrzak)
(Assignee)

Updated

8 years ago
Blocks: 442258
(Assignee)

Comment 9

8 years ago
Created attachment 404531 [details] [diff] [review]
patch, v2.1

Removed bitrot due to bug 509053.
Attachment #403559 - Attachment is obsolete: true
Attachment #404531 - Flags: review?(guy.pyrzak)
Attachment #403559 - Flags: review?(guy.pyrzak)
(Assignee)

Updated

8 years ago
Attachment #403302 - Attachment is obsolete: false

Comment 10

8 years ago
Comment on attachment 404531 [details] [diff] [review]
patch, v2.1

patch itself is ok. I've got some issues with the way you repeat code in the blocks. Font size is 10px on the page which is super small. see attachment
Attachment #404531 - Flags: review?(guy.pyrzak) → review-

Comment 11

8 years ago
Created attachment 408023 [details]
small font on the noneditable page

Updated

8 years ago
Attachment #408023 - Attachment mime type: application/octet-stream → application/image

Updated

8 years ago
Attachment #408023 - Attachment mime type: application/image → image/png

Comment 12

8 years ago
remove font-size:smaller that's what is making the font so small
(Assignee)

Comment 13

8 years ago
Created attachment 408046 [details] [diff] [review]
patch, v3

I fixed all you said.
Attachment #404531 - Attachment is obsolete: true
Attachment #408046 - Flags: review?(guy.pyrzak)

Updated

8 years ago
Attachment #408046 - Flags: review?(guy.pyrzak) → review+

Updated

8 years ago
Flags: approval?
(Assignee)

Updated

8 years ago
Flags: approval? → approval+
(Assignee)

Comment 14

8 years ago
Checking in skins/standard/create_attachment.css;
/cvsroot/mozilla/webtools/bugzilla/skins/standard/create_attachment.css,v  <--  create_attachment.css
new revision: 1.3; previous revision: 1.2
done
Checking in skins/standard/global.css;
/cvsroot/mozilla/webtools/bugzilla/skins/standard/global.css,v  <--  global.css
new revision: 1.68; previous revision: 1.67
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v  <--  filterexceptions.pl
new revision: 1.130; previous revision: 1.129
done
Checking in template/en/default/attachment/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.63; previous revision: 1.62
done
Checking in template/en/default/attachment/show-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/show-multiple.html.tmpl,v  <--  show-multiple.html.tmpl
new revision: 1.27; previous revision: 1.26
done
Checking in template/en/default/flag/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/flag/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.38; previous revision: 1.37
done
Checking in template/en/default/global/textarea.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/textarea.html.tmpl,v  <--  textarea.html.tmpl
new revision: 1.4; previous revision: 1.3
done
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

8 years ago
Keywords: relnote

Comment 15

8 years ago
Added to the release notes in bug 547466.
Keywords: relnote
Blocks: 577881
(Assignee)

Updated

7 years ago
Duplicate of this bug: 364210
You need to log in before you can comment on or make changes to this bug.