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

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, enhancement, P2)

Description

[timeless]

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

Comment 1

[Guy Pyrzak]

not sure why this is a UE thing, seems like a normal bug.

Comment 2

[Frédéric Buclin]

Keep bug 219021 in mind when fixing this bug.

Comment 3

[Frédéric Buclin]

Attachment: 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.

Comment 4

[Frédéric Buclin]

Attachment: 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

[Guy Pyrzak]

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.

Comment 6

[Guy Pyrzak]

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.

Comment 7

[Frédéric Buclin]

(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.

Comment 8

[Frédéric Buclin]

Attachment: 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.

Comment 9

[Frédéric Buclin]

Attachment: patch, v2.1

Removed bitrot due to bug 509053.

Comment 10

[Guy Pyrzak]

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

Comment 11

[Guy Pyrzak]

Attachment: small font on the noneditable page

Comment 12

[Guy Pyrzak]

remove font-size:smaller that's what is making the font so small

Comment 13

[Frédéric Buclin]

Attachment: patch, v3

I fixed all you said.

Comment 14

[Frédéric Buclin]

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

Comment 15

[Max Kanat-Alexander]

Added to the release notes in bug 547466.