Bug 360033 - use of image nodes/blank image (none.png) vs. spans
Summary: use of image nodes/blank image (none.png) vs. spans
Status: RESOLVED FIXED
Alias: None
Product: Orion (Archived)
Classification: ECD
Component: Client (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 0.5   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: accessibility
Depends on:
Blocks: 365361
  Show dependency tree
 
Reported: 2011-10-05 16:08 EDT by Susan McCourt CLA
Modified: 2012-09-25 14:35 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Susan McCourt CLA 2011-10-05 16:08:38 EDT
in bug 349237 I converted most of our icons to sprites.
The command framework still assumes that an image command creates an image node in the document.  While this is true, we need an empty icon to serve as the img src (to prevent broken image boxes on some browsers).  

We could instead create a span and sprite the empty image, but I wasn't sure if there was any code relying on the use of images (such as document.images[]).  (see uses of "imageSprite" class for inlined span images)
Comment 1 Susan McCourt CLA 2011-10-06 13:50:53 EDT
I did some coding down this path for 0.3.
From a command framework point of view, the image node can be gotten rid of.  We would just need to create a span and change our current hover behavior "swap out the image src" to instead change the icon class (from blank to the icon).

The problem is that not all clients use an icon class yet.
- i'm sure i missed places in 0.3.  We can fix these for 0.4

- more importantly, we need to decide how plug-ins such as file commands and editor actions are supposed to describe their icon contributions.  It is convenient to point to a URL on any random site in order to get a good plug-in icon.  I'm not sure how we would otherwise accomplish this, we don't expect plug-ins to supply CSS do we?  

Mark - What does the editor expect for annotation icons contributed by plug-ins?  Do they have to supply CSS or can they contribute a URL to an icon?
Comment 2 Mark Macdonald CLA 2011-10-06 14:51:00 EDT
(In reply to comment #1)
> Mark - What does the editor expect for annotation icons contributed by
> plug-ins?  Do they have to supply CSS or can they contribute a URL to an icon?

Well, plugin contributors can't supply annotations directly right now. There is a limited set of problem severities (warning, error) we understand, which translate into annotation icons, but the translation is done on our side. And it should be easy to change that from <img> to CSS classes -- see AnnotationFactory.showProblems() in editorFeatures.js.

As for plugin APIs in general, I know of one place that expects an image URL, which is the "orion.edit.command" service [1].

> I'm not sure how we would otherwise accomplish this, we don't expect
> plug-ins to supply CSS do we?

If someone's going to contribute several icons from one plugin, sprites would make sense. But we haven't established how/if CSS can be securely contributed -- see Bug 347493.

[1] http://wiki.eclipse.org/Orion/Documentation/Developer_Guide/Plugging_into_the_editor#orion.edit.command
Comment 3 Simon Kaegi CLA 2011-10-06 15:02:25 EDT
make'em give us a data uri?
Comment 4 Susan McCourt CLA 2011-10-06 16:22:07 EDT
> As for plugin APIs in general, I know of one place that expects an image URL,
> which is the "orion.edit.command" service [1].

We do the same for orion.navigate.command

> 
> > I'm not sure how we would otherwise accomplish this, we don't expect
> > plug-ins to supply CSS do we?
> 
> If someone's going to contribute several icons from one plugin, sprites would
> make sense. But we haven't established how/if CSS can be securely contributed
> -- see Bug 347493.

right.  That's what I'm wondering about.  If we were to change our current API from 
  image: someURL
to
  imageClass: some css class that uses a data URI or a sprite

then we are assuming they are contributing css.

For this reason I think that the command framework will continue to use HTML image elements to represent images, so that we have the option to use src=
"somePluginURL".  It's really the simplest thing for a plug-in to do, and you could even use an icon from some other domain.

By using "none.png" (empty image) then we can use CSS tricks (spriting or data URI's) in addition to good old fashioned image URL's.

Does anyone think this is the wrong answer?
Comment 5 Simon Kaegi CLA 2011-10-06 20:54:06 EDT
I think this is the right direction. The only concern I have with someone supplying a url is that a plugin provider could use it to track users even though they're not activating a plugin. It also slows down rendering if we have to go elsewhere to get an image where the server may or may not provide appropriate cache headers.

That's why I "seriously" prefer a data uri approach. I wonder if we can fetch the image and make a data uri in the browser...
Comment 6 Susan McCourt CLA 2011-10-06 22:45:33 EDT
interesting.  That keeps the API simple (just give us a URL).  We'll do a get and make data and pass it on.  Will investigate for 0.4
Comment 7 Susan McCourt CLA 2011-12-01 15:44:17 EST
(In reply to comment #0)
> in bug 349237 I converted most of our icons to sprites.
> The command framework still assumes that an image command creates an image node
> in the document.  While this is true, we need an empty icon to serve as the img
> src (to prevent broken image boxes on some browsers).  
> 
> We could instead create a span and sprite the empty image, but I wasn't sure if
> there was any code relying on the use of images (such as document.images[]). 
> (see uses of "imageSprite" class for inlined span images)

Turns out there is an accessibility angle here as well.
As long as we are using image nodes, we can supply alt text.  This is required for accessibility.

In places where we have gotten rid of image nodes (nav columns, etc.) and used spans instead, we may find ourselves going back to image nodes to get the alt text.  For example, the file/folder icons in the navigator.  If there is no alt text, there is no way for a screen reader to tell the user they are looking at a file vs. folder.

(Note this doesn't mean we can't sprite the image, it just means we would do what the command framework does now, which is use an empty image and alt text, and sprite the background.)

I'm going to remove the milestone on this bug for now.  The accessibility question suggests we shouldn't rush to get rid of image nodes (and therefore none.png) but instead should leave it around until we fully understand the requirements.  Generalizing title because this bug might end up meaning "go back to using image nodes instead of spans" rather than "get rid of image nodes"
Comment 8 Carolyn MacLeod CLA 2011-12-02 03:04:26 EST
(In reply to: comment 5 and comment 6)
Not sure if the following is useful for generating data uri's. On this page:
http://www.websiteoptimization.com/speed/tweak/inline-images/
It says:
This code reads the image and converts it to base64 automatically at the server.
<?php echo base64_encode(file_get_contents("../images/folder16.gif")) ?>
You pay for this editing convenience with some server-side processing.
Comment 9 Carolyn MacLeod CLA 2011-12-02 03:16:11 EST
Just an FYI on the high contrast problem with css background images:
http://dojotoolkit.org/community/a11yHighContrastMode
And of course, Susan mentioned that the screen reader side of the accessibility equation is to have alt text.
So a rule of thumb (whether using data uri's or sprites) would be: 
- use img tag, with alt text, for all images that convey meaning
- ok to use css background for images that are only decorative, or you can use img with alt="" for decorative images
Comment 10 Susan McCourt CLA 2012-09-25 14:35:15 EDT
Marking resolved for several reasons:
- we barely use image only commands anymore.
- we need to accept URL's from extension points.
- we have done the accessibility pass for the image nodes (as well as attaching the right attributes on span nodes).

So there's no reason to convert the current image nodes to anything else at this time.