Closed Bug 240903 Opened 20 years ago Closed 20 years ago

Clicking image input highlights the image in purple/blue

Categories

(SeaMonkey :: General, defect, P2)

Product:
SeaMonkey
Component:
General
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla1.7final

People

(Reporter: quatrixj, Assigned: roc)

References

(
URL
)

Details

(Keywords: fixed1.7, regression, Whiteboard: [Comment 23: short-term done; long-term wanted...] fixed-aviary1.0)

Attachments

(3 files, 4 obsolete files)

Screen shot from https://www.schwabplan.com/login/login.asp?wci=participantLogin
2.49 KB, image/gif
Details
Screen shot from Yahoo! Driving Directions
20.20 KB, image/gif
Details
Screen shot from http://www.ati.com/support/driver.html
5.38 KB, image/gif
Details 
User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a) Gecko/20040418
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a) Gecko/20040418

On https://www.schwabplan.com/login/login.asp?wci=participantLogin and other
pages with inputs of type "image", clicking the image button highlights the
image in purple.

Reproducible: Always
Steps to Reproduce:
1. Open https://www.schwabplan.com/login/login.asp?wci=participantLogin
2. Click the "LOGIN" button (login.gif).

Actual Results:  
The image is highlighted before the next page loads.

Expected Results:  
I previously saw a light grey outline around a clicked image, without the image
itself changing color.

The problem started in Mozilla 1.7b.  When I uninstalled it and reinstalled 1.6,
the problem went away.  When I again installed 1.7b, the problem reappeared. 
I'm currently on nightly build 2004041809, and the issue has not yet been fixed.
 Anton van Bohemen reported a similar problem in Bug 224798, Comment #3.

I'm running Windows 98 SE with an ATI Radeon 8500.
Open http://maps.yahoo.com/dd, enter any two addresses, and click somewhere on
the resulting map.
Keywords: regression
The problem seems to be that clicking the image selects it, and selected images
get a purple overlay like that....

If someone can track down the exact date when this regressed, that would be most
helpful.
Keywords: qawanted
Seeing it with Linux gtk2 build for some time, too.
The earliest report of this bug so far is in Build 2004031616 (from James Arnold
 Bug 224798, Comment #4). The bug doesn't appear in 1.6 final, the roadmap says
1.6 was realeased on 20040115. That's about a 2 month period. Anyone running any
builds in between these dates can help out by narrowing the window.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 98 → All
Bug? I thought that this was a new feature added to make it easier to see when
an INPUT image had been clicked.  I quite like it the way that it is now :)
> 1.6 was realeased on 20040115

But it branched on 2003-12-12.  So three-month period.
re: comment #6
This is a problem for some cases, e.g. if the submit target is another frame
(like in bug 224798) then the selected image remains purple even when the form
has been submitted.

Is there an RFE for this somewhere?

re: comment #7
Borris indeed you are right.
If this is indeed a new feature then it should be easy to trace right?
3 months means somwhere on the order of 1000 checkins or more that could be
responsible.  If the regression range is gotten down to a matter of days,
tracing should be possible.
*** Bug 241129 has been marked as a duplicate of this bug. ***
Actually, testing shows that this started between 2004-02-27-09 and
2004-02-28-08.  Most likely due to the changes from bug 53966....
Interesting. Unfortunately I know almost nothing about the selection code.
Maybe it's a matter of calling changing the click event status to handled when
we submit?
That sounds like a good idea.
Doesn't seem to help...

Also, it looks like when I start dragging out of an <input type="image"> the
nsTransferableFactory::Produce method in nsContentAreaDragAndDrop is not getting
called. For that matter, neither is nsContentAreaDragDrop::DragGesture.  I'm not
sure whether that's related to this bug, but it may be.
Note that in a build in which this "works" if I click on an image input and then
drag off somewhere, it often becomes selected when I drag dar enough.. (eg on
the page in comment 0, drag straight down).
I can add another dimension to this one.

When you have two <input> images together, the first seems to have the 'purple'
(although it looks more blue to me) fault and the second one looks fine.

Demonstated well here: http://tom.playford.net/imagebug/

Tested on firefox 8 and Mozilla 1.6, both linux.

Tom
On that page, I see both images turn blue when clicked.
About purple vs. blue -- in Windows the highlight color depends on your Control
Panel > Display > Appearance settings.
Summary: Clicking image input highlights the image in purple → Clicking image input highlights the image in purple/blue
This is a quite visible regression, and it is new since 1.6 so I am asking for
blockage of 1.7. This is more important than ever since so many products will be
based on 1.7.
Flags: blocking1.7?
any thoughts on what it might take to fix this?  roc, any recommendations on
what we should do for 1.7?
I need to investigate it, I guess.
Assignee: general → roc
Priority: -- → P2
So I was poking at the code in nsFrame::HandlePress and it special-cases
draggable objects (like <img>, <a>, etc) for selection extension purposes.  It
forgets <input type="image">.  (It also screws up <area>, since it tries to QI
to nsIDOMHTML_Anchor_Element for that... but anyway).

Short-term, we could just hack <input> in there...  Medium-term, we could use
the nsIImageLoadingContent attribute biesi's adding in bug 196380.  Long-term, I
would think that all this logic should live in the dragdrop service and this
code should just ask said service, "Is this node draggable?"

If the "long-term" fix is not too hard, we should just do it...

That's all for the first place HTMLImageElement is used in that code.  The
second place is editor-specific and should probably just use nsIImageLoadingContent.

And no matter what, we should fix the Area thing.
Depends on: 196380
Depends on: 244859
For 1.7, we should definitely just hack it in there, and also put that patch on
the trunk for consistency.
Yeah, makes sense.  We should probably just spin off a bug for moving this logic
out of here (into nsContentUtils or something).
Flags: blocking1.7? → blocking1.7+
Whiteboard: working on patch
is a patch close?  trying to wrap 1.7 in the next few days...
I won't be able to create and test a patch here anytime soon (we're talking June
7 soon)... Given the information in comment 23, anyone who knows C++ and has a
bit of time should be able to patch this and do some testing, though.

Comment on attachment 149677 [details] [diff] [review]
(Av1-r) <nsFrame.cpp> 'short-term' (for review only)


I have no compiler: Could you compile/test/review this patch ? Thanks.
Attachment #149677 - Attachment description: (Av1-r) <nsFrame.cpp> → (Av1-r) <nsFrame.cpp> 'short-term'
Attachment #149677 - Flags: review?(roc)
Attachment #149677 - Attachment description: (Av1-r) <nsFrame.cpp> 'short-term' → (Av1-r) <nsFrame.cpp> 'short-term' (for review only)
The whole point is that the compile/test part is the time-consuming part....
(In reply to comment #30)
> The whole point is that the compile/test part is the time-consuming part....

I do know, but what else can I do ?
I'd like to know someone who could do this part(s) on my patches, but I don't :-(

Serge, is there a reason why you can't get a compiler?
(In reply to comment #32)
> Serge, is there a reason why you can't get a compiler?

Unfortunately, there is:

To make the story short:
My computer is not suited for a compilation environment;
and I don't plan to upgrade in the near future :-(

This is why I'm looking for compile/test partner(s) ... And I may just have
found one today :-)
testing patch on windows.
I need to add:
#include "nsUnicharUtils.h"
#include "nsIDOMHTMLInputElement.h"

to get this to compile. I'll post a package of my build to run performance tests
with in case there are extremely negative side effects.
There really shouldn't be any perf impact from that patch, as far as I can tell.
Test zip file will appear at
http://ftp.mozilla.org/pub/mozilla.org/mozilla/nightly/experimental/mozilla-win32-240903.zip
in approximately fifteen minutes (ftp mirrors will need to sync)

I verified that the behaviour is fixed, i will attach the patch with the added
includes for review.
Attachment #149677 - Flags: review?(roc)
Attachment #149770 - Flags: review?(roc)
Av1b,
with duplicated |#include "nsCOMPtr.h"| removal.
Attachment #149770 - Attachment is obsolete: true
Comment on attachment 149771 [details] [diff] [review]
(Av1c-r) <nsFrame.cpp> 'short-term' (for review only)
[Check in: See Av1c-ci]

It seems that leaf and I have been posting updated patch at the same time...

Leaf, maybe you could try another compilation for this one ? (if needed)
Attachment #149771 - Flags: review?(roc)
Attachment #149770 - Attachment description: nsFrame.cpp diff (same as previous, but with necessary includes) → (Av1b) nsFrame.cpp diff (same as previous, but with necessary includes)
Attachment #149770 - Flags: review?(roc)
Attachment #149770 - Attachment description: (Av1b) nsFrame.cpp diff (same as previous, but with necessary includes) → (Av1b-r) nsFrame.cpp diff (same as previous, but with necessary includes)
Keywords: qawanted
Hardware: PC → All
Comment on attachment 149771 [details] [diff] [review]
(Av1c-r) <nsFrame.cpp> 'short-term' (for review only)
[Check in: See Av1c-ci]

Looks good!
Attachment #149771 - Flags: superreview+
Attachment #149771 - Flags: review?(roc)
Attachment #149771 - Flags: review+
Comment on attachment 149771 [details] [diff] [review]
(Av1c-r) <nsFrame.cpp> 'short-term' (for review only)
[Check in: See Av1c-ci]

'approval1.7=?':
blocking1.7+; Simple U.I. code fix, low risk.
Attachment #149771 - Flags: approval1.7?
224798 may also be related
Av1c-r, with white space diff included.

(Modified lines move one column to the left, as this part of the file is off by
+1.)
Comment on attachment 149771 [details] [diff] [review]
(Av1c-r) <nsFrame.cpp> 'short-term' (for review only)
[Check in: See Av1c-ci]

a=chofmann for 1.7
Attachment #149771 - Flags: approval1.7? → approval1.7+
Whiteboard: working on patch → [Comment 23: short-term done; long-term wanted...]
Blocks: 224798
Checked in on trunk and branch.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
I guess we should file a separate bug on the "long-term" fix in comment 23?
Keywords: fixed1.7
Attachment #149776 - Attachment description: (Av1c-ci) <nsFrame.cpp> 'short-term' (for check-in only) → (Av1c-ci) <nsFrame.cpp> 'short-term' (for check-in only) [Checked in: Comment 46]
Attachment #149776 - Attachment is obsolete: true
Attachment #149771 - Attachment description: (Av1c-r) <nsFrame.cpp> 'short-term' (for review only) → (Av1c-r) <nsFrame.cpp> 'short-term' (for review only) [Check in: See Av1c-ci]
Attachment #149771 - Attachment is obsolete: true
Target Milestone: --- → mozilla1.7final
Whiteboard: [Comment 23: short-term done; long-term wanted...] → [Comment 23: short-term done; long-term wanted...] needed-aviary1.0
Installed nightly build 20040602 and it looks good, thanks.
I am still seeing this behavior on Aviary/Firefox. Should I open a Fx bug for
that, wait patiently for this fix to make its way there, or what?



Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040604
Firefox/0.8.0+
the answer is B - wait patiently for this fix to make its way there.

That's what the "needed-aviary1.0" is for in the status whiteboard. At some
point the firefox folks will copy/port the 1.7 fixes into aviary.
Whiteboard: [Comment 23: short-term done; long-term wanted...] needed-aviary1.0 → [Comment 23: short-term done; long-term wanted...] fixed-aviary1.0
*** Bug 243242 has been marked as a duplicate of this bug. ***
Attachment #166613 - Attachment description: Screen shot from http://www.ati.com → http://www.ati.com/support/driver.html
I'm seeing a related issue on a few web sites.  Go to
http://www.ati.com/support/driver.html and click the "Go" button in the
bottom-right.  The resulting highlight around the button is much more obvious
than the normal one.  I can reproduce in Firefox 1.0 (Mozilla/5.0 (Windows; U;
Win98; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0) but it looks okay in Mozilla
1.8a4 (Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a4) Gecko/20040927).  Is
this a regression of 240903, something new, or was a fix not ported to Firefox?
 Screen shot is attached.
> Is this a regression of 240903, something new, or was a fix not ported to
> Firefox?

Most likely the last.  If it works on trunk, it'll work in Firefox 1.1.
Attachment #166613 - Attachment description: http://www.ati.com/support/driver.html → Screen shot from http://www.ati.com/support/driver.html
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: