Closed Bug 255378 Opened 20 years ago Closed 20 years ago

Click-and-hold on subsequent lines of wrapped links fails to invoke context menu

Categories

(Core Graveyard :: Widget: Mac, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: waynegwoods, Assigned: roc)

References

Details

(Keywords: regression)

Attachments

(2 files, 2 obsolete files)

User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a3) Gecko/20040811 Firefox/0.9.1+ Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a3) Gecko/20040811 Firefox/0.9.1+ I've only tested this in Mac OS X so far. I'll test it out in Windows tomorrow. On the Mac, if you click-and-hold on a link, it brings up a context menu with options such as "Open Link in New Window "(same as right click on Windows). In recent builds, if the link is long and wraps over two or more lines, then hold-clicking on any line of the link *except* the first line often fails to bring up the context menu. Hold-clicking on the first line of the link will always bring up the menu, however. If you've brought up the context menu by hold-clicking on the first line, then it usually works on subsequent lines for a while. If you change the focus, say by highlighting a word, then the subsequent lines become unresponsive once again. This regression occurred between the 20040714 and 20040715 builds, at least in Firefox. The problem exists in Seamonkey as well, however. I'll attach a simple test case so the problem can be tested easily. Reproducible: Always Steps to Reproduce: 1.Navigate to a page with a link that is fairly long (say, a whole sentence). See the attached test case. 2.Shrink the browser window until the link is forced to wrap over more than one line. 3.Click and hold on the second line of the link. 4.Observe that no context menu appears (hopefully). If the menu does appear, then dismiss it and highlight a section of text that is outside the link, and try again.
Open the attachment, then shrink the browser window to be sure that the link wraps over multiple lines. Click-and-hold on any line except the first one and observe the lack of context menu. If it does appear, then try highlighting some text and then try again.
> This regression occurred between the 20040714 and 20040715 builds What are the timestamps on those builds?
I was never sure how to determine the timestamp for a Fx build, as it doesn't list it in the about: info. Where can I find it? I can download a few Seamonkey builds and test them out, anyway.
It's not in the URL bar? What about the filename on the FTP site?
I presume you mean the title bar, which is where Seamonkey lists build id? If so, Firefox doesn't do that. The FTP file never lists build id either. I'm not sure where else to look. Would there be some way of telling by looking at the Tinderbox for those dates, or something similar?
> The FTP file never lists build id either. Er... Not if you look in latest/, but if you look in the dated directories...
Does the ftp directory name match the true timestamp, though? I remember looking at Seamonkey builds in the past, and the 2 digits after the date for the directory didn't match the 2 digits after the date that appeared on the title bar.
It's usually within an hour or two.. That usually gives a window of 27-28 hours, which is a lot less than the 48 you get with just dates.
Okay, I'll download them again just to be sure and test them out tonight (tonight being another 6-7 hours away for me). I'll download some Seamonkey builds from the same time period and note the timestamps for all. Incidentally, right-clicking in Windows XP doesn't produce the same bug, so it looks like it's Mac-specific.
Okay, the last trunk builds to work properly were: Firefox 2004071409 Mozilla 2004071408 The first ones to exhibit this bug were: Firefox 2004071509 Mozilla 2004071508 The Mozilla Seamonkey timestamps were taken from the title bar, and the Firefox timestamps were taken from the directory name. Of interest, I tried a number of branch builds of both Seamonkey (1.7 branch) and Firefox (0.9 branch), and none exhibit this bug. The latest branch build I tested was the 0.9.3 release of Firefox. So whatever checkin caused this regression, it was only checked into the trunk.
Of the checkins in the relevant range (http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-07-14+08%3A00%3A00&maxdate=2004-07-15+08%3A00%3A00&cvsroot=%2Fcvsroot), the ones that look like they may be responsible are: bug 65917, bug 151375 (the latter is what I really suspect). Wayne, could you test something for me? Remove the *|*:-moz-any-link:focus { -moz-outline: 1px dotted invert; } rule from your ua.css file (with Mozilla shut down) and retest this bug? Does that help any?
Keywords: regression
Sounds like reflowing the element because it got focus is killing the hold. I wonder why, no frames should be destroyed.
> Sounds like reflowing the element because it got focus is killing the hold. I > wonder why, no frames should be destroyed. We have to reframe when the outline gets drawn, because there's no other change hint to force creation of a view without doing that. That's also causing bug 257672. In that bug Roc said I was wrong about the fix, but I still think it would be fixed by bug 249102
(In reply to comment #14) > No, we don't force view creation. > http://lxr.mozilla.org/mozilla/source/content/shared/src/nsStyleStruct.cpp#641 My understanding is that we would just force view creation / removal when an outline appears/disappears, but the only way to do that right now is to force frame reconstruction. I thought that was the idea of fixing bug 249102, although now that I read nsChangeHint.h I'm not sure that's what nsChangeHint_ForceFrameView is supposed to do (http://lxr.mozilla.org/seamonkey/source/content/shared/public/nsChangeHint.h#50) > 50 // TBD: add nsChangeHint_ForceFrameView to force frame reconstruction if the frame doesn't yet > 51 // have a view
We don't force frame reconstruction either. Read the code. All we do is a reflow.
I'm sure the click-hold thing has something set up to make it go away if the mouse moves... could the reflow be synthesizing mouse move events or something? ccing pinkerton, since he knows something about how click-hold works, iirc.
I couldn't find the ua.css file (assuming the test is still useful at this point). Where's it kept?
i forget if the click hold is stopped by a mouse move alone. it is stopped by a synthesized drag_gesture event which is made up from mouse moves. all the click-hold code is in the ESM, it's not very complicated. basically an event handler and a timer.
(In reply to comment #19) > I couldn't find the ua.css file. Where's it kept? res/ua.css in the Mozilla install dir.
Removing: *|*:-moz-any-link:focus { -moz-outline: 1px dotted invert; } from ua.css does indeed fix the problem, in both Mozilla and Firefox.
Product: Browser → Seamonkey
Setting a blocking request just to make it visible again :) The cause of this bug has been known for quite a while now, see comment 22, so hopefully someone knows what to do. :)
Flags: blocking-seamonkey1.0a?
Oh, man. First, this is in the wrong product. Second, the cause is NOT known. Just removing that CSS is not an option, and the CSS is not the cause of the bug. The cause of the bug is something in the interaction between the click-hold listener and layout and we (or at least I) don't really know what.
Assignee: general → joshmoz
Component: General → Widget: Mac
Flags: blocking-seamonkey1.0a?
Product: Mozilla Application Suite → Core
QA Contact: general → mac
Nominating for the relevant near-term releases (most importantly, the Gecko 1.8 nomination).
Flags: blocking1.8b2?
Flags: blocking-aviary1.1?
Flags: blocking1.8b2?
Flags: blocking1.8b2-
Flags: blocking-aviary1.1?
Flags: blocking-aviary1.1+
Flags: blocking1.8b3+
Attached patch fix (obsolete) (deleted) — Splinter Review
Similar fix to what we did to make dragging work in the face of frame destruction/reconstruction (which happens here because of inline frame pullup/pushback). In fact, we can reuse most of that work and simplify things a lot. Two notes: using mPresContext is fine, we cannot be dispatching events to content that's not in this ESM's document. I'm changing the behaviour of drag events so that they specify the keyboard modifiers at the time of original mousedown, but I think that actually makes more sense than using the modifiers from when the mouse first moved enough for a drag to start.
Assignee: joshmoz → roc
Status: NEW → ASSIGNED
Attachment #181004 - Flags: superreview?(bzbarsky)
Attachment #181004 - Flags: review?(bzbarsky)
Comment on attachment 181004 [details] [diff] [review] fix >Index: content/events/src/nsEventStateManager.cpp > nsEventStateManager::KillClickHoldTimer() > } // KillTooltipTimer Want to fix that comment? >+nsEventStateManager::FillInEventFromGestureDown(nsMouseEvent* aEvent, >+ nsIView* view = mCurrentTarget->GetClosestView(&targetToView); >+ nsPoint viewToWidget; >+ nsIWidget* widget = view->GetNearestWidget(&viewToWidget); >+ NS_ASSERTION(widget == aEvent->widget, "Widget confusion"); >+ aEvent->point = refPointTwips - (targetToView + viewToWidget); This is putting the point in the coord system of the _frame_ we're dispatching to, no? Is that really right? Is that fixed up somewhere else to the correct view's coord system? I know you just copied this code, but it seems fishy. Possibly separate bug fodder. >Index: content/events/src/nsEventStateManager.h >+ void FillInEventFromGestureDown(nsMouseEvent* aEvent, PRBool aIsTrusted); Document a bit, please. r+sr=bzbarsky with that.
Attachment #181004 - Flags: superreview?(bzbarsky)
Attachment #181004 - Flags: superreview+
Attachment #181004 - Flags: review?(bzbarsky)
Attachment #181004 - Flags: review+
Attached patch fix #2 (obsolete) (deleted) — Splinter Review
Here's the new patch incorporating your comments. I've fixed the coordinate translation.
Attachment #181004 - Attachment is obsolete: true
Attachment #181075 - Flags: superreview?(bzbarsky)
Attachment #181075 - Flags: review?(bzbarsky)
Attached patch oops (deleted) — Splinter Review
That patch enabled click-hold context menus on all platforms, we really don't want that :-).
Attachment #181075 - Attachment is obsolete: true
Attachment #181076 - Flags: superreview?(bzbarsky)
Attachment #181076 - Flags: review?(bzbarsky)
Attachment #181075 - Flags: superreview?(bzbarsky)
Attachment #181075 - Flags: review?(bzbarsky)
Comment on attachment 181076 [details] [diff] [review] oops r+sr=bzbarsky
Attachment #181076 - Flags: superreview?(bzbarsky)
Attachment #181076 - Flags: superreview+
Attachment #181076 - Flags: review?(bzbarsky)
Attachment #181076 - Flags: review+
Comment on attachment 181076 [details] [diff] [review] oops This fixes a 1.8b3 blocker
Attachment #181076 - Flags: approval1.8b2?
Attachment #181076 - Flags: approval1.8b2? → approval1.8b2+
checked in
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
I believe this checkin is responsible for Camino crashing bug 299419.
Depends on: 299419
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: