Closed Bug 623763 Opened 9 years ago Closed 9 years ago

Link/tap highlighting performance has regressed

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: stechz, Assigned: mbrubeck)

References

Details

(Keywords: perf)

Attachments

(1 file)

I don't have data, but it looks like link highlighting has regressed. It used to show instantly on the front page, for instance.

We can close as WONTFIX, but filing this just so we can make sure nothing obvious has regressed.
(front page==start page)
Summary: Link highlighting has regressed → Link highlighting performance has regressed
Keywords: perf
Attached patch Patch v1Splinter Review
We removed the mousedown listener here. This puts it back.
Assignee: nobody → wjohnston
Attachment #501874 - Flags: review?
Attachment #501874 - Flags: review? → review?(21)
Bug 619824 added Browser:MouseOver and move the Browser:Highlight there.
Bug 609866 removed Browser:MouseDown

This patch adds Browser:MouseDown back and puts the Browser:Hightlight back in it.

NOTE: Let's make sure we are not regressing these other bugs
I don't like to do highlighting on the MouseDown event, in fact in a pre-e10s world I've opened a bug to add a delay to highlight but the resulting code has been removed during the merge.

I think we should not highlight anything until a small delay, to not highlight thing during a pan action or a pinch zoom.
Comment on attachment 501874 [details] [diff] [review]
Patch v1

I'm fine with the code but as I've said in the previous comment I don't think this is the right way to go.
Attachment #501874 - Flags: review?(21) → review-
(In reply to comment #4)
> I think we should not highlight anything until a small delay, to not highlight
> thing during a pan action or a pinch zoom.

I disagree.  I never hold down my finger for a significant time when tapping links, so the delay means that highlights never appear for me anymore.  This is much more annoying to me than a brief flicker of highlight before panning.

I think that I must have a fairly different "touch" from Vivien when tapping the screen; this could also explain the 200ms touch threshold from bug 547722 which never made much sense to me.
(In reply to comment #6)
> (In reply to comment #4)
> > I think we should not highlight anything until a small delay, to not highlight
> > thing during a pan action or a pinch zoom.
> 
> I disagree.  I never hold down my finger for a significant time when tapping
> links, so the delay means that highlights never appear for me anymore.  This is
> much more annoying to me than a brief flicker of highlight before panning.

oh? We could probably reduce the delay or we could send the highlight feedback on the MouseUp action so you will see the highlight on a quick click even before the delay is finished.

> 
> I think that I must have a fairly different "touch" from Vivien when tapping
> the screen; this could also explain the 200ms touch threshold from bug 547722
> which never made much sense to me.

You're right, I'm not really quick when touching my screen phone :)
Assignee: wjohnston → mbrubeck
Blocks: 619824
Summary: Link highlighting performance has regressed → Link/tap highlighting performance has regressed
I think we should show the tap hightlight as soon as possible. The feedback is important to users. When I see a link get clicked, but no tap highlight, I think there is a bug or that fennec is "slow".

I'd rather see a tap highlight and then dismiss it when we start panning, then never see it at all.
Comment on attachment 501874 [details] [diff] [review]
Patch v1

After talking about this a bit on IRC, we want to try it and see how it feels in a nightly.
Attachment #501874 - Flags: review- → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Verified fixed on build: Mozilla /5.0 (Android;Linux armv7l;rv:6.0a1) Gecko/20110516 Firefox/6.0a1 Fennec/6.0a1 
Device: LG Optimus 2X (Android 2.2)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.