142042 - Image right-click Create Link is inconsistent with other ways of adding a link

Status: RESOLVED FIXED

(SeaMonkey :: Composer, defect)

Description

[Robin Whittle]

When right-clicking an image, one option is "Create Link".  The problem is that
this creates a different dialog box from that which is used for all other ways
of adding a link:

   1 - Right-click image, select "Image and Link Properties" and 
       then use the Link tab.

   2 - Double-click image and then use the Link tab.

   3 - "Format > Image and Link Properties" and then use the Link tab.

The three "normal" methods above result in a dialog box which has an option
called "Show border around linked image".  But the dialog box which results from
"Right-click > Create Link" does not have this option.

This is a separate matter from the following problems, but I will mention them
here anyway:

1 - Bug 99541 where it is noted that "Create Link" is available even if 
    the image has a link already.  Also, how it might be best to replace it
    with "Remove Link".

2 - A forthcoming bug from me regarding borders around images after a link
    has been added.  I will mention it here when I create it.

Comment 1

[Robin Whittle]

I just created bug 142093:

  Border display in image with link has two anomalies

This is about what the unhappy outcome when Composer does not specify 
a border width for an image which has a link.  This will occur if the 
user adds the link with "Right-click > Create Link", or if one of the 
other three methods is used, and the "Show border around linked image" 
option is ticked, or left in its default ticked state.

Comment 2

[Kathleen :Brade]

-->cmanske

Comment 3

[Robin Whittle]

I just checked build 2002071208 on Win2k and the problem is unchanged from how 
I described it on 3 May.

  - Robin

Comment 4

[Charles Manske]

The "Link" tab in the image dialog is there as a convienece to make it easier
to create a link around an image. If you want to create a link to an image 
using the context menu, simply insert the image first. I think it would 
overly complicated the create link menuitem and dialog to always include all the
image attributes for simple text link creation.

Comment 5

[sujay]

bulk verification.

Comment 6

[Robin Whittle]

I respectfully request that this bug be re-opened.   As far as I can
see, my concerns remain valid and have not been addressed in what
Charles wrote when he changed it to "Resolved - Invalid".


I can't be sure, but I suspect Charles has misunderstood my concerns.  
Perhaps he hasn't and we simply have a different view on how things
should be.  Here I will try to restate, more directly, what I am
concerned about.  Please remember this is from the point of view of the
naive user, who does not know or care about the internal structure of
the program and who may have different understandings about what
particular menu items are supposed to do than Charles or other
developers may understand.

I will refer to the behavior of the most recent Mozilla build 2002071508
which behaves the same as builds such as 2002050908 from when I wrote
this bug.  I found that 2002071208 was lacking menu items and
double-click actions described below. 


From the user's point of view, there are four ways of adding a link to
an existing image.  (For the purposes of this test, I added an image
with "Insert > Image" and then chose a relative location, and added a
short Alt text.)

   1 - Right-click image, select "Image and Link Properties" and 
       then use the Link tab.

   2 - Double-click image and then use the Link tab.

   3 - Select the image and then "Format > Image and Link Properties" 
       and then use the Link tab.

          (These three evidently lead to the same dialog box.)

   
   4 - Right click the image and select the "Create Link" option.

          (This leads to a very different dialog box.)


To me, if I right click an image which has no link, and there is a menu
item "Create Link" then it seems to me that this menu item is there to
add a link to this image.  (Subtleties such as the distinction between
"adding" or "creating" a link are not on my mind.)  Assuming I am right
about the function of this menu item, then the problems are:

1 - The 4th approach leads to a different different dialogue box than 
    the other three approaches - which is a matter of consistency
    in user interface.

2 - That the dialogue box resulting from the fourth approach lacks
    functions, including the "Show border around linked image" option.

    This can lead the user into creating a file which has border 
    problems as discussed in bug 142093.


I still think that this situation is a bug.

Perhaps, if you believe that the "Create Link" option in the
right-click-image menu is a totally different beast from what you
understand as the proper approach to adding a link to an image (one of
the first three options, including "Right-click > Image Properties")
then this is not a bug.  But I argue that for the average user, the
presence of a "Create Link" menu item when right-clicking a link-less
image looks like a legitimate way of adding a link to this image.   This
is reflected in bug 99541 :

   After creating a link in composer and right clicking it, in the 
   pop-up menu it gives an option to create a link even though there is 
   a link already in place.

This bug 99541 seems to have received little attention and no action
since it was written in September 2001.   

So in summary, I think that either:

1 - The image-right-click menu option "Create Link" should be removed
    or renamed or something. Or:

2 - It should be made to:

    a - Lead to the same dialogue box as the three other methods
        mentioned above.  And:

    b - It should not appear for an image which already has a link.


Here is Charles' response on the current bug, in which he marked it as
being "Resolved - Invalid" and part of his response to bug 142093 which
seems to further explain his thinking.  (sujay then changed "Resolved"
to "Verified" with resolution still being "Invalid".)

Charles - sorry to seem pedantic, but I only have these words to go on
and I am trying to figure out if you understand my point in this bug and
disagree, or whether our understandings differ in some way.


Charles wrote:

> The "Link" tab in the image dialog is there as a convienece 
> to make it easier to create a link around an image. 

Yes, I agree.  This is part of the dialog box which appears in the first
three approaches listed above.


> If you want to create a link to an image using the context menu, 
> simply insert the image first. 

Here we get into pedantic interpretation of words . . .   "Link" is a
pesky word because "adding a link to an image" in the way I have been
using it means "modifying the image definition so it contains a
hyperlink".  I am avoiding the use of the word "to"!   Where the link
points to is not an issue in this discussion.   "Adding a link to an
image" could only really mean my meaning above, since a link cannot
point to an image, unless the image has a target - and in that case the
link would point to the target.

This sentence of Charles' seems to indicate that he thinks I am
referring to creating a hyperlink somewhere which will point to the
image.  This is not the case at all.  I am discussing modifying the
image definition so there is a hyperlink pointing to something else
which will be followed by the browser when the image is clicked.

This makes me think that Charles misunderstood what I wrote.


> I think it would overly complicated the create link menuitem 
> and dialog to always include all the image attributes for 
> simple text link creation.


This makes me think that Charles' understanding of the "Create Link"
item in the right-click menu is a generic item for creating a "text
link".  Indeed, right clicking when nothing is selected brings up a
dialog box asking for the text and the link location.  Right clicking
some selected text and then using "Create Link" brings up a box asking
only for the link location, which is the same as what happens when doing
this to an image.

So I think that Charles' understanding of this right-click "Create Link"
menu item is as a generic way of creating a link.  Whereas my
understanding is that this is one of several ways of adding a link to an
existing image, and therefore should be consistent with the other three
ways of doing it.

I suspect this shows a discrepancy between what individual developers
think particular menu items should do and what some or many naive users
would think it is for.

In bug 99541, Charles wrote:

> As I explained in bug 142042, I don't think supporting insertion 
> of an image in the Link dialog is necessary. If you want to 
> create a linked image, use the Image dialog.

This again makes me think that he misunderstands the point of my bug. 
He seems to think that I am discussing inserting an image, but I am
not.  I am discussing an existing image where the user wants to make
that image have a hyperlink pointing somewhere.

 - Robin

Comment 7

[Charles Manske]

So what you are asking that if you right-click on an image (or click on
"Link Properties" button or menuitem when selected item is an image) we should 
bring up the Image Properties set it to the "Link" tab instead of the separate
"Link Properties" dialog. That seems reasonable.

Comment 8

[Kathleen :Brade]

agreed; it is inconsistent to have "create link" show link dialog when context
clicking on an image.

Comment 9

[Robin Whittle]

Yes, Charles, what you wrote above would be good.  This would make all four ways 
of adding a link to an image behave the same way, with all the same functions, 
such as the option about border size.

Also, as per bug 99541, there should not be a "Create Link" when right clicking 
an image which already has a link - there should be a "Remove Link" (not "Remove 
Links") menu item.

 - Robin

Comment 10

[Charles Manske]

Attachment: patch v1.

When selected item is an image, bring up Image dialog with Link panel selected.

Comment 11

[[e-mail]]

Comment on attachment 91511 [details] [diff] [review]
patch v1.

>+    var isImage = (element && element.nodeName.toLowerCase() == "img") ? true : false;
Oh no, you didn't really write "? true : false" did you?
>+    if (isImage)
if (element && element.nodeName.toLowerCase() == "img") will do here.

>+  gDialog.linkTab          = document.getElementById("imageLinkTab");
> 
>   // Get a single selected image element
>   var tagName = "img";
>+  var linkTab = document.getElementById("imageLinkTab");
Eh?

>   if ("arguments" in window && window.arguments[0])
>   {
>     imageElement = window.arguments[0];
>     // We've been called from form field propertes, so we can't insert a link
>     var imageLinkTab = document.getElementById('imageLinkTab');
>     imageLinkTab.parentNode.removeChild(imageLinkTab);
Re-use gDialog.linkTab here.

Comment 12

[Charles Manske]

Attachment: patch v2

Address reviewers comments.

Comment 13

[[e-mail]]

Comment on attachment 91826 [details] [diff] [review]
patch v2

r=[e-mail]

Comment 14

[Alec Flett]

Comment on attachment 91826 [details] [diff] [review]
patch v2

sr=alecf

Comment 15

[Asa Dotzler [:asa]]

Comment on attachment 91826 [details] [diff] [review]
patch v2

a=asa (on behalf of drivers) for checkin to 1.1

Comment 16

[Charles Manske]

Fixed 1.1 trunk