Closed Bug 404438 Opened 17 years ago Closed 17 years ago

Location bar drop-down does not collapse on second click on down-arrow

Categories

(Firefox :: Address Bar, defect, P2)

Product:
Firefox
Component:
Address Bar
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
Firefox 3

People

(Reporter: shishki, Assigned: Gavin)

References

(
URL
)

Details

Attachments

(2 files, 1 obsolete file)

patch
4.23 KB, patch
Gavin
: review+
Details | Diff | Splinter Review
followup patch
1.15 KB, patch
enndeakin
: review+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007111905 Minefield/3.0b2pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007111905 Minefield/3.0b2pre Location bar drop-down does not disappear on second click Reproducible: Always Steps to Reproduce: 1. Start Minefield 2. Click on the down arrow in the URL bar 3. Click on it again Actual Results: Drop-down list remains Expected Results: Drop-down list disappears
Version: unspecified → Trunk
i've seen this sometime, so i can somehow confirm (even if it's random happening)
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007111913 Minefield/3.0b2pre ID:2007111913 this sometimes happens, unreproducable though
Status: UNCONFIRMED → NEW
Ever confirmed: true
i guess i figured it out. this is probably the behavior when double-clicking the locationbar dropdown. but it's a bug nonetheless.
i can "reproduce" this when one of the tabs is loading/reloading (I use the Tinderbox page for that coz it takes at least 10 secs to load)
should i file a bug for the doubleclick behavior?
Severity: normal → minor
Flags: blocking-firefox3?
I can't reproduce on current Mac trunk. WFM? Clearing nom for now, please renom if this can still be reproduced.
Flags: blocking-firefox3?
Keywords: qawanted
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007120708 Minefield/3.0b2pre Still reproducible.
Flags: blocking-firefox3?
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007120705 Minefield/3.0b2pre Still reproducible. Like a month ago.
It only happens once per browser session AFAIK; not repeatable in the same session. Also when you dismiss the pop-up by clicking in the page it doesn't happen anymore. Although typing in the address bar does not influence the reproducibility.
(In reply to comment #10) > It only happens once per browser session AFAIK; not repeatable in the same > session. > Yes, >Steps to Reproduce: >1. Start Minefield ...
Seth, related to the hiding trick, perhaps?
Flags: blocking-firefox3? → blocking-firefox3+
Priority: -- → P3
Target Milestone: --- → Firefox 3 M11
> Seth, related to the hiding trick, perhaps? no, I don't think so. but I'm glad you cc me, I have in my notes: "log bug: click history toggle second times sometimes re opens" My gut tells me the problem might be here: <binding id="history-dropmarker" extends="chrome://global/content/bindings/general.xml#dropmarker"> <implementation> <method name="showPopup"> <body><![CDATA[ var textbox = document.getBindingParent(this); textbox.showHistoryPopup(); ]]></body> </method> </implementation> <handlers> <handler event="mousedown" button="0"><![CDATA[ this.showPopup(); ]]></handler> </handlers> </binding> Not sure yet mousedown is showPopup() (and not toggleHistoryPopup()). Let me do some digging.
Assignee: nobody → sspitzer
what I'm seeing is the mousedown, then the popuphidding, then the mousedown (almost like we are not consuming the roll up event?) note, after I initially open the browser window, I can reproduce this. when I click twice (even with a delay). but after that, it doesn't happen and I only get the mousedown and popuphiding (but not the second mousedown). cc'ing johnath, who has fought similar battles with Larry before.
zoinks, fix in hand, I think. we need to call setConsumeRollupEvent() before openPopup(), not after.
Status: NEW → ASSIGNED
that fixes it, patch coming. I think we want to fix search.xml as well, but I've never seen the bug there (gavin, have you?) If this fix gets r=+, you will want to fix http://lxr.mozilla.org/mozilla/source/xpfe/components/autocomplete/resources/content/autocomplete.xml#1583
URL: http://lxr.mozilla.org/mozilla/search...
This reminds me of bug 401712, maybe we can fix them both at once?
Comment on attachment 293250 [details] [diff] [review] patch for autocomplete.xml and search WhatGavinSaid
Attachment #293250 - Flags: review?(gavin.sharp)
Seth, thanks for the heads-up, but I don't think we have a problem. We have three cases and the behaviour within each case is always consistent: 1. In-page autocomplete consumes the rollup event. 2. Location bar autocomplete doesn't consume the rollup event 3. History dropmarker doesn't consume the rollup event, except when clicking on the history dropmarker, where a timeout hack is used to ignore the click.
(In reply to comment #20) > Seth, thanks for the heads-up, but I don't think we have a problem. I haven't been following along closely here, but doesn't Seth's patch suggest that you must now call setConsumeRollupEvent before openPopup for it to take effect? If that's true, http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/xpfe/components/autocomplete/resources/content/autocomplete.xml&rev=1.159&mark=1581,1583#1567 does seem buggy, unless it's expected that the call doesn't affect the first opening.
OK, so it turns out that our popup isn't in a popupset and therefore it consumes rollup events by default. See nsMenuFrame::ConsumeOutsideClicks().
> This reminds me of bug 401712, maybe we can fix them both at once? I'll go fix that bug and attach a new patch. but to clarify my understanding, bug #401712 doesn't have anything to do with this bug, right? (It just makes sense to fix both at the same time.)
Actually, it does have something to do with this bug. We need to do something like: this.popupBoxObject.setConsumeRollupEvent( this.mInput.consumeRollupEvent ? Ci.nsIPopupBoxObject.ROLLUP_CONSUME : Ci.nsIPopupBoxObject.ROLLUP_NO_CONSUME); we don't always want to consume the roll up event (like we would have done, if I just reordered the calls). Otherwise, I would have regressed #340131 – Takes two clicks to go to pasted URL. Note, that bug isn't really fixed on the trunk either, except for the first time, because it relies on the out-of-order nature of the calls (that I'm fixing.) Note, fixing this bug will also fix bug #340839: "Location bar dropdown that is opened by text inputting does not disappear on click the dropdown button" fix coming that will address this bug, bug #340131, #340839 and bug #401712
Comment on attachment 293616 [details] [diff] [review] patch note, while this fixes bug #340131 on windows, it does not fix that bug on mac, but neither did the original patch. I'll take the mac issue off into bug #340131 (as a spin off.)
Attachment #293616 - Attachment description: patch, still testing... → patch
Attachment #293616 - Flags: review?(gavin.sharp)
Attachment #293616 - Flags: review?(gavin.sharp) → review+
Seth, do you want me to land this for you?
yes, please! I didn't realize I had r/a to land. It's been a while, so can you double check and retest before landing? thanks gavin.
Assignee: moco → seth
Status: ASSIGNED → NEW
over to gavin sharp.
Assignee: seth → gavin.sharp
(I thought for sure I had commented here a few days ago, but I guess it got lost in one of my sessions.) While this does improve the situation by making the location bar consistent between the first and second opening using the dropmarker, the behavior here still isn't quite right. It looks like the code is trying to allow the "click button again to close" behavior by calling setConsumeRollupEvent(CONSUME) when you open the menu with the dropmarker, and then returning to NO_CONSUME when you close it. The problem is that it only re-sets to NO_CONSUME in closePopup, which isn't called when you dismiss the popup with a click somewhere else. This leads to inconsistent behavior. It sounds like we should try to implement the "click dropmarker to close" behavior without messing with the consumeRollupEvent.
This is a really annoying bug, is this getting fixed?
and whats about fixing? ;(
I landed this patch: mozilla/browser/components/search/content/search.xml 1.121 mozilla/toolkit/content/widgets/autocomplete.xml 1.128 I'm going to post a followup patch to fix the problems I describe in comment 30.
Severity: minor → normal
Status: NEW → ASSIGNED
Keywords: qawanted
OS: Windows XP → All
Priority: P3 → P2
Hardware: PC → All
Target Milestone: Firefox 3 beta3 → Firefox 3
Depends on: 425798
Attached patch followup patch — — Splinter Review
The bug that's left is: 1) open history dropdown with dropmarker 2) close popup by clicking somewhere else in the window 3) type something in the URL bar to make the autocomplete popup appear, click toolbar button Expected: toolbar button activated (event isn't consumed) Actual: toolbar button not activated (event is consumed) This fix (and the fix for bug 414845) depend on the fact that showHistoryPopup only exists for autocomplete.xml's nsIAutocompleteInput impl. We need to reset mConsumeRollup to false when the dropmarker-opened popup is closed, since it should only ever be true while the history popup is open. This makes us match IE, and fixes the bug described above.
Attachment #293616 - Attachment is obsolete: true
Attachment #312363 - Flags: review?
Attachment #312363 - Flags: review? → review?(enndeakin)
Attachment #312363 - Flags: review?(enndeakin) → review+
No longer blocks: 340839
Landed the followup: mozilla/toolkit/content/widgets/autocomplete.xml 1.130
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Verified fix on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008050806 Minefield/3.0pre.
Status: RESOLVED → VERIFIED
Attachment #293616 - Attachment is obsolete: false
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: