Closed Bug 222861 Opened 21 years ago Closed 5 years ago

Add ability to show attachment graphics in comment text

Categories

(Bugzilla :: User Interface, enhancement)

2.17.4
enhancement
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1472522

People

(Reporter: bugreport, Unassigned)

References

Details

(Whiteboard: bz_extension)

Attachments

(1 file)

I am considering the merits of extending auto-linkification of attachments so
that  attachments with a mime type of image/* could be refernced in a comment by
mentioning attachment xxxx,70% and having that cause the comment history to
generate an <img src=> reference to view the attachment at 70% of browser width
in the example here.

My thoughts are to only do this for non-obsolete comments of the appropriate
mime-types.

I think the correct thing to do is to always add the image AFTER the end of the
current line (and give it its own <P>).

I am not sure if this would cause problems with bandwidth for dial-up users and
how we could prevent problems with pr0n. (user pref - show me graphics??) 

  Copying gerv, myk, and justdave for input.
What purpose would this feature serve? I haven't heard anyone asking for it...

Gerv

It is frequently requested in engineering environments.

Joel: can you give a concrete example? Do you mean engineering as in bridges and
girders?

> I am not sure if this would cause problems with bandwidth for dial-up users and
> how we could prevent problems with pr0n. (user pref - show me graphics??) 

A thought: how would a pref prevent pr0n problems for people with graphics
turned on?

Gerv
This is communications engineering.  Often, we have problems in voice quality
(score) versus packet loss or jitter where the best way to describe a bug is to
show the chart (image).  My users find it to be a real pain to jump up and back
to attachments and would rather have them inline.

Seems like it could be useful, but I'm not fond of making the commenter specify
the image size.  Bugzilla should determine some reasonable maximum size,
probably the width of the view pane and no more than half its height (so the
image can be viewed in context with preceding and succeeding comments).  Users
can "View Image" to see it in its full glory.

Speaking of full glory (aka porn), the feature makes the problem only marginally
worse.  In some ways it makes it better, since while more users will see an
image they'd rather not see, they'll alert the Bugzilla admin sooner, making
public Bugzilla installations less of a target for porn distributors.

Regarding dial-up users, they already have a browser preference for browsing
with images disabled; so I don't think this needs a Bugzilla preference.
Also, we wouldn't want the text "attachment xxxx" to automatically include the
image - I can see that getting annoying quite quickly, if you were just talking
about the attachment a lot.

(I'm still not convinced of the significant value of inline images over
attachments, though.)

Gerv
Actually, on my site, I use the term "figure XXX" to mean it should be inlined.
I guess I did a bad search, Bug 252782 
I have a one line patch there.
(In reply to comment #1)
> What purpose would this feature serve? I haven't heard anyone asking for it...
> Gerv

http://projects.pyerotechnics.com/show_bug.cgi?id=123
*** Bug 252782 has been marked as a duplicate of this bug. ***
Blocks: 252782
going to do this in two parts:

1) adding the support for #2, this patch
2) decision support for inlining images, bug 252782
Attachment #160844 - Flags: review?
if debate continues on this, assign to me.
Actually, you may as well take it.  While I would like to see this happen, I am
too busy to drive it now.   

We do have at least 2 examples of sites that did decide this is worth having. 
To avoid additional debate, this should have a system param controlling it so
that sites that dont want it are not forced to take it.  The user-level pref can
wait and be added later.
Assignee: bugreport → jpyeron
good idea, i thought it but did not write it.

the sysparam will default to 'on' for new installs
but the patch will default it to 'off'

dito on the user pref too.

but this will all be in bug 252782, see comment 11.

lets get this one, an innocuous one, rolled in to CVS.
Jason: that's just a bug where a graphic is attached. A single click allows you
to see what it is. That's not really inconvenient.

Having graphics inline means that everyone who views the bug has to download it,
whether they are interested in it or not. It means it's possible for some idiot
on DSL to attach a 2MB graphic to a bug and make in unusable for people on modems.

Gerv
But shouldn't the guy on modem not be browsing with images on anyway? Is there a
way to specify in a user-defined CSS file that we don't want to load images?
That would certainly make a lot of sense..
kiko: I assume your double negative was meant to say "why isn't he browsing with
images off"? Because people don't do that - they just sit and wait. And even on
my DSL line, which is only 512Kbits and which I'm also using for other things, I
don't want to be downloading large graphics in every other bug either.

The amount of coding effort you'd need to put into this (ways to turn it off,
ways to not inline too-big stuff, ways to restrict the width and height,
whatever) vastly outweighs the minor convenience of not having to make a single
click to see the image.

Gerv
but this effort is a one time effort, which developers like myself are willing 
to do.

If the patch defaults to inline:off for existing/new sites and existing users,
then what would the "harm" be.

It would require the admin to inline=on first
then the user could inline=off.

> The amount of coding effort you'd need to put into this (ways to turn it off,
> ways to not inline too-big stuff, ways to restrict the width and height,
> whatever) vastly outweighs the minor convenience of not having to make a
> single click to see the image.

Comment on attachment 160844 [details] [diff] [review]
bugzilla-222861-v1.01.diff adds mime type info to the mix

This needs to be combined with bug 252782, needs to inline images only when
there is some indication that the attacher/commenter intended for this
particular image to be inlined, the size of the image is less than the system
limit (which would be zero on any system Gerv uses), and the user has not
elected to supress the inline images (be means of a cookie - most likely)
Attachment #160844 - Flags: review? → review-
> and the user has not
> elected to supress the inline images (be means of a cookie - most likely)

Bugzilla doesn't require cookies, so you can't use a cookie. (No, saying that
people who switch cookies off have to have inline images isn't fair :-). If we
are going to go down the route of allowing different users to customise the UI
different ways (which, IMO, we shouldn't, it's a nasty slippery slope), then we
need to have a preferences table. (Adding new columns to the profiles table for
each isn't really feasible either.)

It wouldn't need to be complicated: user_id, preference_id and value.

Gerv
I agree.  We could have a cookie that, if not present, means no images (even if
we attempt to set it for users by default) or a userpref.

Actually, keeping userprefs VERY expandable would be a good thing.  I have a
very similar requirement to provide a set of selectable form simplifications
which would be good fodder for userprefs but I currently do with cookies.
As I mentioned in IRC the other day, I'm OK with this feature provided it's off
by default, administrators can specify a size limit when turning it on, and we
*don't* add a user preference for it.  The first two features can be handled by
a single param that sets the size limit, with 0=off.

Regarding the user preference, there are several client-side options available
to users who want to override the default, and they should avail themselves of
them.  If that proves insufficient, then we can consider a user preference, but
that shouldn't be part of the initial implementation.
Comment 22 sounds good.  We can easily add a userpref later if we need to.  In
particular, a site that needed that level of control could very easily add it
with a simple patch plus a very quick template hack.  In 2 years live with an
old version of this, I have not needed to do so.


QA Contact: mattyt-bugzilla → default-qa
I apologize, as this post/comment is the same that I posted in bug 252782.  I just noticed that the two are dependent on each other, and that this one actually more pertains to my problem/request.

The last comment on this bug is dated 2004-10-11.  I was just wondering if there's been any progress on it since then. I'm very interested as I'm looking for an easy way of attaching images/screenshots to text fields in Testopia.  Currently, the only option is to add an attachment first, and then reference to that attachment in the text field by modifying the HTML source code.  I'm hoping for an easier solution. Thanks...

P.S. Should I create a new bug (enhancement request) against this under Testopia section?
Assignee: jpyeron → ui
There is a patch for this bug on bug 252782, however, this enhancement has been deemed as extension worthy and I'm not sure what the procedure is for having extensions reviewed and approved.
Whiteboard: bz_extension
(In reply to comment #26)
> There is a patch for this bug on bug 252782, however, this enhancement has been
> deemed as extension worthy and I'm not sure what the procedure is for having
> extensions reviewed and approved.

  There's no review or approval process. You just create them and link them from the Bugzilla:Addons wiki page.
posted extension to github find it here http://github.com/pyrzak/Bugzilla-Extension--Inline-Images

It should have zero impact on any user who doesn't like images inline. This version is super simple no attempts to make thumbnails or remember the state of show/hide inline images based on the cookie. But it is enough to see how it might work and hopefully some folks will be interested in this.
(In reply to comment #28)
> posted extension to github find it here
> http://github.com/pyrzak/Bugzilla-Extension--Inline-Images

  Very cool. Make sure to add this to:

  https://wiki.mozilla.org/Bugzilla:Addons#Bugzilla_Extensions

  so that people can find it! :-)

Done in BMO, will be part of Bugzilla 6.0.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.