Closed Bug 730952 Opened 12 years ago Closed 12 years ago

LayerController mayHaveTouchListeners isn't updated correctly

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 13

People

(Reporter: wesj, Assigned: wesj)

Details

Attachments

(4 files)

We update this when we receive notification from a page that it has listeners (i.e. usually when they're attached during page load) and during handleLocationChange in GeckoApp. handleLocationChange isn't fired when we switch or close the active tab though.

While looking into this I decided that I hated the OnTabsChangedListener on GeckoApp because it fires for all sorts of changes but never explains why, so I'm cleaning it up a bit and moving it into Tabs. I also made a million (4) patches to test the lucasr method.
Assignee: nobody → wjohnston
Attachment #601027 - Flags: review?(margaret.leibovic)
Attachment #601029 - Flags: review?(margaret.leibovic)
Attachment #601030 - Flags: review?(margaret.leibovic)
This guy does the real work of fixing mayHaveTouchListeners. I removed our current special message in handleLocationChange because its being handled by the listener for STOP (LOADED often fires before STOP). I left in special code that runs when we get the message from HasTouchListeners message from Javascript, because that might fire long after a page load ends.
Attachment #601031 - Flags: review?(margaret.leibovic)
Should note this is split off from bug 725458. The testcase there is good for seeing this problem. Or heck, just open google maps in one tab, and something non-touchy in another. Switching back and for the state is not updated.
Comment on attachment 601027 [details] [diff] [review]
Patch 1/4 - Move TabsChangedListener to Tabs.java

>diff --git a/mobile/android/base/Tabs.java b/mobile/android/base/Tabs.java
>--- a/mobile/android/base/Tabs.java
>+++ b/mobile/android/base/Tabs.java

>+        Iterator<OnTabsChangedListener> items = mTabsChangedListeners.iterator();
>+        while (items.hasNext()) {
>+            items.next().onTabsChanged(tab);
>+        }
>+    }

For extra nice clean-up, you could do:

        for (OnTabsChangedListener listener : mTabsChangedListeners) {
            listener.onTabsChanged(tab);
        }


>diff --git a/mobile/android/base/resources/layout/tabs_row.xml b/mobile/android/base/resources/layout/tabs_row.xml
>--- a/mobile/android/base/resources/layout/tabs_row.xml
>+++ b/mobile/android/base/resources/layout/tabs_row.xml

Get rid of this :)
Attachment #601027 - Flags: review?(margaret.leibovic) → review+
Attachment #601029 - Flags: review?(margaret.leibovic) → review+
Attachment #601030 - Flags: review?(margaret.leibovic) → review+
Attachment #601031 - Flags: review?(margaret.leibovic) → review+
Inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c303fb97006

(grr... now I see I forgot that clean up bit)
https://hg.mozilla.org/mozilla-central/rev/8c303fb97006

I assume the pushed patch is a coalesce
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 13
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: