Closed Bug 1058027 Opened 5 years ago Closed 5 years ago

about:home is unresponsive to hardware menu button on returning to main activity on 2.3 devices

Categories

(Firefox for Android :: General, defect)

31 Branch
ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 35
Tracking Status
firefox31 --- affected
firefox32 --- affected
firefox33 --- affected
firefox34 --- affected
firefox35 --- verified
fennec + ---

People

(Reporter: domivinc, Assigned: domivinc, Mentored)

References

Details

Attachments

(1 file, 6 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140716183446

Steps to reproduce:

Tested on HTC Desire Z (Android 2.3.3) with Firefox version 31.0

Steps to reproduce:
1-Start Firefox (and open about:home)
2-Click on the hardware Menu button to open the options menu
3-Select “...” on the menu to get more options
4-Select “Settings”
5-Do nothing in the Settings page, just click on the Move Back button 
6-Click again on the hardware Menu button



Actual results:

A) you have to wait 5 seconds (a very long time) to get back to the home screen between the steps 5 and the step 6. 

B) after 5 seconds, you get back to the home screen but in a very bad state: the hardware Menu button doesn’t work anymore.



Expected results:

Options menu should open again
To be able to open again the menu, the user can change of tab in the home screen or go to another page in order to fix the issue and have again a working hardware Menu button.

The issue doesn’t occur if you are on a web page, it’s linked to the about:home page.

I didn’t test the issue on a device without a hardware Menu button.
Sorry about that. We fixed this in Firefox 32, currently in Firefox Beta on Google Play.
Status: UNCONFIRMED → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1030113
Thanks for the quick answer.
I just installed on my device the Firefox Beta (32) from Google Play, and I still have the same issue.
I also tested it with a Release version of the code.

1- The fact that it doesn't work on my device could be linked to the Android version 3.3.2 in my case, and 4.2 in the bug 1030113 ? 

2- I had a quick look to the code patch for bug 1030113 (https://hg.mozilla.org/mozilla-central/rev/3d7306c70b1e), and I see nothing that should solve the issue.
In my case I never used an icon to open the menu, I always use the hardware Menu button (the 3 dots icon is not displayed on my device). 
The proposal fixe in 1030113 seems to implement an icon change and remove a 4.0 hack:

" -     * This is an ugly hack for 4.0+ Android devices that don't have a dedicated menu button"

What I understand from this code patch, is that the menu will never be accessible for the 4.0+ Android devices that don't have a dedicated menu button. With the previous hack, they can use the three dots button, but now the "..." won't be displayed and they don't have a hardware menu button. The situation will be worse on such tablet without hardware button. It could be nice to make a test of version 32 with a tablet that don't have a dedicated menu button.
Let's request info from the the author
Flags: needinfo?(liuche)
tracking-fennec: --- → ?
I can only repro on a 2.3 device, but I see it on 31+. It seems to be unrelated to bug 1030113, though.

It looks like after opening any new activity (Settings, or Sync Setup from the banner), about:home no longer handles the hardware button correctly. (I'm not sure if this happens on 2.3 devices without hardware buttons, because I couldn't find any to test on.)

I'm not sure what the regression range is, but it seems like it's been around for a while :/ I'm not sure what feature this could be related to - any ideas?

Dominique, as for your question about menu buttons:

> The situation will be worse on such tablet without hardware button. It could be
> nice to make a test of version 32 with a tablet that don't have a dedicated
> menu button.

The code for bug 1030113 is actually for handling the menu items in Search Settings, so this is unrelated to the main menu item for settings in the main browser. (We have some fragmentation in our Settings code for pre/post-v11, so GeckoPreferenceFragment handles post-v11 devices for settings, so you won't see the code for menu items in GeckoPreferences. See
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/preferences/GeckoPreferenceFragment.java#62 )

Menu button code for the main browser app is handled here, if you were curious: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/toolbar/BrowserToolbar.java#490
Flags: needinfo?(liuche)
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: DUPLICATE → ---
Summary: Cannot open Settings twice from the about:home page using hardware menu button → about:home is unresponsive to hardware menu button on returning to main activity on 2.3 devices
Attached patch Proposed patch (obsolete) — Splinter Review
This bug is linked to 1003123 and the patch fix both issues.

Context menu and hardware menu button are connected to the layerView visibility. If the layerView is not visible when the activity is re-started, something is broken and the menus won’t work properly. I didn’t find the exact reason: events are not correctly routed, focus is missing? An expert eye could probably get a more accurate explanation to know if the reason is in the Firefox code or on the Android side. 

It’s the reason why both issues occur only in about:home and not when a web page is displayed.

The patch change the visibility of the layerView for a short period of time when the HomePager is visible. 
I limited the fix to Gingerbread phones. Bug 1030113 (Android 4.2) won’t be affected by this fix.
Attachment #8478679 - Flags: review?(liuche)
This looks like a pretty bad perf bug for 2.3 because re-launching Fennec from the background is also really, really slow, like 5-6 seconds on an 2.3 SII.

We should try to figure out why this is happening and fix it at the root. Since this seems to be happening only on about:home, we should start looking at that code to see if somehow our onPause/onStop code is breaking somewhere.

Changing the default home page doesn't fix the problem, so it's not tied to Top Sites. Perhaps the HomePager? Perhaps we're overriding something that is v11+ specific while backgrounding/restarting, so 2.3 devices are not handling backgrounding correctly.

Good eye on noticing that the menu button is linked to LayerView visibility, Dominique, and thanks for the patch! I don't think that we should just try to fix this on resume though, but rather find out what's causing the problem. If you'd be interested in digging into this bug, I'm happy to mentor.
Comment on attachment 8478679 [details] [diff] [review]
Proposed patch

Clearing review for now, because we should try to fix the underlying problem.

Drive-by: refreshForHomePager should be called in onStart instead of onResume because this only occurs when the activity is completely hidden (not just partially obscured, by a dialog for example).
Attachment #8478679 - Flags: review?(liuche)
Mentor: liuche
OS: Windows 8.1 → Android
Hardware: x86_64 → ARM
Attached patch Another proposal patch (obsolete) — Splinter Review
Another way to fix the issue is to avoid to hide the layerView on Gingerbread devices.

I didn’t detect any visual change with this second patch, it looks like the function showHomePagerWithAnimator doesn’t work on my device. I tried to increase the delay of the animation but I didn’t see any change. It could be a specific behavior on my HTC Device? 

This second patch fixes also the bug 1003123.
Attachment #8479026 - Flags: feedback?(liuche)
I still didn't understand why I cannot see any animation but I found the main reason to hide the layerView: it's to prevent screen reader access. The layerView is already not "visible" on the screen probably behind the HomePager View, but screen reader can always read the content.  

The menu functionality has been broken when this code (hideWebContent, hideWebContentOnPropertyAnimationEnd, ...) has been implemented to really hide the layerView using: mLayerView.setVisibility(View.INVISIBLE);

We could perhaps find a way on Gingerbread to stop the screen reader access when the HomePager is on top of the layerView?
Comment on attachment 8479026 [details] [diff] [review]
Another proposal patch

Review of attachment 8479026 [details] [diff] [review]:
-----------------------------------------------------------------

Drive-by: While I haven't tested this to ensure this is the case specifically on 2.3, we can't hide the layer view because we'll likely break accessibility: screen readers will read both the HomePager content and the web content hidden below the HomePager. Additionally, we'll cause overdraw issues.

Did you know that the options menu pathways on pre-Honeycomb and Honeycomb and up are different? See options menu vs. context menu in [1]. This distinction might explain our issue. I think you should make sure BrowserApp.onCreateOptionsMenu/openOptionsMenu/etc. are being called at the appropriate times and with the appropriate actviity state. 

[1]: https://developer.android.com/guide/topics/ui/menus.html
Attachment #8479026 - Flags: feedback-
(In reply to Dominique Vincent from comment #9)
> We could perhaps find a way on Gingerbread to stop the screen reader access
> when the HomePager is on top of the layerView?

This may be possible, but you still face overdraw issues (when the web content is drawn, then the HomePager is drawn on top of it - since the web content is never seen, it is an inefficient drawing cycle): I'd prefer a different solution.
Mentor: michael.l.comella
Attached patch patch1058027_3.diff (obsolete) — Splinter Review
Last status:

1- I made another verification regarding the calls to the different menu functions (onCreateOptionsMenu, openOptionsMenu, …) and I didn’t notice any problem on this part.
I compared the trace between the different cases (on Home, on a web page, on Home after a display of the settings), there is no difference.

2- When the options menu is not visible (second use of the hardware menu button on Home page after a settings display), in fact the menu is open: you can touch the right bottom side area of the screen (“...” button is not visible but here), then imagine the position of the “Settings” item and touch here, then you will see that the settings activity is open. So in fact the menu works but the pixels of the menu are not visible. It confirms also the point 1: there is no difference in the calls of the Menu functions. It looks like that the screen is locked in a state where there is no redraw of the elements but the elements are here and you can click on the elements. 

3- I found another way to fix the issue without changing the visibility of the layerView to avoid the screen reader problem. I found that when the HomePager is touched (move, select another tab, …), the screen is “unlocked” and there is no more issue with the different menus (options or context menus). When the HomePager is moved, in fact the screen is probably invalidated. The patch attached implements this fix. Bug 1003123 is also fixed by this patch.


When another activity is open, the BrowserApp is stopped but the screen of the BrowserApp is locked in a bad state if the layerView is not visible. Then, when the BrowserApp is restarted, the screen is still locked. It is a kind of screen overlay that makes the display changes invisible, but this overlay accepts the touch events. I think that something is not done correctly when the activity is paused/stopped with the layerView not visible. Any ideas?
Assignee: nobody → domivinc
tracking-fennec: ? → +
Comment on attachment 8480239 [details] [diff] [review]
patch1058027_3.diff

Review of attachment 8480239 [details] [diff] [review]:
-----------------------------------------------------------------

It'd be good if we can figure out exactly why the menu gets disabled, but your guess is as good as mine. Provided we comment the code, I'm fine with this hackish approach, but let's try to find one that is less likely to have strange edge case issues, like scrolling.

Unrelated to this code specifically, when you upload a patch, make sure to flag a reviewer by clicking "Details" (next to the patch), selecting "?" from the appropriate category's dropdown, and filling in the reviewer's email. This field should auto-complete, which you can do most easily by ":<irc-nick>" because the colon simplifies the auto-completion.

::: mobile/android/base/BrowserApp.java
@@ +672,5 @@
> +        // (see bug: 1058027 and 1003123).
> +        if (Versions.preHC && isHomePagerVisible()) {
> +            // scroll the content, it will invalidate the view
> +            // mHomePager.invalidate(); // it works randomly
> +            // mHomePager.setCurrentItem(0); // it works if the current tab index is not 0

How about `mHomePager.setCurrentItem(mHomePager.getCurrentItem());`? Then we don't have to deal with weird, messy scroll code.
Attachment #8480239 - Flags: feedback+
(In reply to Michael Comella (:mcomella) from comment #13)

> How about `mHomePager.setCurrentItem(mHomePager.getCurrentItem());`? Then we
> don't have to deal with weird, messy scroll code.

I already tried : mHomePager.setCurrentItem(mHomePager.getCurrentItem());
It doesn’t work, because there is a test to make nothing when the tab is already active...

I have more details about the bug. It’s really linked to the visibility of the mLayerView.
When Firefox starts for the first time, the visibility of mLayerView is by default set to VISIBLE. Gecko starts and is connected with mLayerView that is visible, many messages are sent/received between mLayerView and Gecko. At one point, the HomePager decides to hide mLayerView to prevent screen reader access. It seems to be ok for Gecko (I’m not sure that it is 100% safe). 
Now the activity is paused by another activity. When the system comes back to Firefox, the activity is resumed. Gecko was loaded, so some messages are sent/received between the LayerView and Gecko but one of the message cannot be correctly manage by the system because the visibility of mLayerView is set to INVISIBLE. In fact the situation is really bad because the layerView is not visible (probably no surface) but Gecko tries to draw on it.
The last message sent is in the methods updateCompositor/resumeCompositor.

At this point the system fails 5 s:
1- The end of the previous Activity is not correctly managed by Android (no stop and no destroy of the Preferences activity for instance). This point is really not good because it could occur the same to any activity started by the user from other applications.
2- The screen surface is not refreshed.  

I will try to trace updateCompositor/resumeCompositor . It’s probably here that something really bad happens.

Now regarding the hackish approach, I would vote for my first patch: change the visibility of mLayerView a short period of time when the activity is resumed. In fact a surface is probably set in the mLayerView,  Gecko is happy even if the mLayerView is hidden 100ms later... It's already what is done when Firefox starts for the first time.
Attached patch patch1058027_4.diff (obsolete) — Splinter Review
I tried to fix the issue on the correct side: when the mLayerView is set to VISIBLE/INVISIBLE. The surface linked to mLayerView must be set with the same state.
I still have something that I cannot explain, the mLayerView cannot be hidden the first time (even if there is no page/url to display in the surface). I’m going to investigate on this part now. The first time the mLayerView is hidden occurs perhaps too early and Gecko doesn’t manage the situation correctly. I will try to get an answer.

The patch patch1058027_4.diff must be tested carefully on all the devices. I didn’t limited the fix to the Gingerbread devices. But we can easily add a test to implement the fix only for Gingerbread.
Attachment #8481286 - Flags: review?(michael.l.comella)
I'm unfamiliar with the the gecko message passing/layerView entanglements (which sounds fishy; potentially buggy?) so snorp, do you have any ideas what might be going on here?
Flags: needinfo?(snorp)
Attached patch patch1058027_5.diff (obsolete) — Splinter Review
The last Gecko message sent when the activity is paused, is: COMPOSITOR_PAUSE.
I verified, the message is also sent when the bug occurs.
I also compared the other messages when the activity is paused and restarted and I didn’t find any missing messages or differences.

My conclusion of the different tests I made, is that one surface View seems to not be correctly destroyed when the mLayerView is INVISIBLE.
I tried different solutions to fix the issue. The best solution (that seems to work in all the cases) is to change the visibility of the mLayerView when the activity is paused. It solves both bugs  1058027 and 1003123 with a limited code change (patch1058027_5.diff).
Attachment #8481892 - Flags: review?(michael.l.comella)
Comment on attachment 8481892 [details] [diff] [review]
patch1058027_5.diff

Review of attachment 8481892 [details] [diff] [review]:
-----------------------------------------------------------------

Drive-by: I think this should be in onStop, not onPause - onStop is called when the entire Activity is obscured, whereas onPause is if anything else partially obscures the Activity (a dialog, for example).

Otherwise, looks good to me. I tried this with some accessibility turned on, and there didn't seem to be any problems with the wrong layerview being read.
Attachment #8481892 - Flags: feedback+
Attached patch patch1058027_6.diff (obsolete) — Splinter Review
Thanks for the code review.
I made the move of the code to the "onStop" method (see attached patch #6).
Attachment #8483471 - Flags: review?(liuche)
Comment on attachment 8483471 [details] [diff] [review]
patch1058027_6.diff

Review of attachment 8483471 [details] [diff] [review]:
-----------------------------------------------------------------

I apologize for the delay.

I really don't like the idea of displaying the LayerView underneath the HomePager for overdraw reasons so I'd like to figure out what the cause of this is. Thanks for all of your thorough research thus far.

I noticed that, for me on my Motorola device, the animation returning from Settings to the main activity looks unexpectedly different after this patch, namely, the Fennec activity seems to "reappear" twice. Notably, when this patch is not applied, the same "reappearance" occurs, but only when the view is scrolled (which you mentioned fixes the issue). Do you have any idea why this might be?
Attachment #8483471 - Flags: feedback+
(In reply to Michael Comella (:mcomella) from comment #20)
> I really don't like the idea of displaying the LayerView underneath the
> HomePager for overdraw reasons

And accessibility reasons, but I can't verify that this patch breaks this because talkback seems broken on my 2.3 devices.
Attachment #8479026 - Attachment is obsolete: true
Attachment #8479026 - Flags: feedback?(liuche)
Attachment #8481286 - Attachment is obsolete: true
Attachment #8481286 - Flags: review?(michael.l.comella)
Attachment #8481892 - Attachment is obsolete: true
Attachment #8481892 - Flags: review?(michael.l.comella)
(In reply to Michael Comella (:mcomella) from comment #20)
> Comment on attachment 8483471 [details] [diff] [review]
> patch1058027_6.diff
> 
> Review of attachment 8483471 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> 
> I really don't like the idea of displaying the LayerView underneath the
> HomePager for overdraw reasons so I'd like to figure out what the cause of
> this is. Thanks for all of your thorough research thus far.

In the last patch (_6.diff) I found a way to limit this overdraw: I set the visibility in the onStop method. The layerView is going to be visible but in fact the activity is already hidden by another one. 


> 
> I noticed that, for me on my Motorola device, the animation returning from
> Settings to the main activity looks unexpectedly different after this patch,
> namely, the Fennec activity seems to "reappear" twice. Notably, when this

On my HTC with the last patch, it doesn’t “reappear” twice. The activity view is animated from the left to the right side (same animation between activities). Are you sure that you are using the last patch (_6.diff)? This patch doesn’t change the onResume method.


> patch is not applied, the same "reappearance" occurs, but only when the view
> is scrolled (which you mentioned fixes the issue). Do you have any idea why
> this might be?

When the reappearance doesn’t occur (without the patch) the surface view is probably a wrong surface. I think that the surface is not redrawn in this case, it’s probably a cached surface. It’s probably the reason why the menu cannot be visible on this cached surface.

I made some accessibility tests. TalkBack version 3.5.1 works on my 2.3 device (HTC Desire Z). I didn’t notice any issue on my device. 
When the onStop method is called, the layerView is set to VISIBLE, but the activity is no more visible. So it’s not an issue for the accessibility.
When you come back to the activity, if you trace the onResume method, we could have an accessibility issue at this point because the layerView is visible. But in fact there is no issue because the layerView is visible but is totally white. The last web page is not visible on the layerView.
(In reply to Dominique Vincent from comment #22)
> On my HTC with the last patch, it doesn’t “reappear” twice. The activity
> view is animated from the left to the right side (same animation between
> activities). Are you sure that you are using the last patch (_6.diff)? This
> patch doesn’t change the onResume method.

Yeah, I used the latest patch. Typically the animations between devices of different manufacturers are different - I wonder if the animations on my device just make this affect more noticeable. 

> I made some accessibility tests. TalkBack version 3.5.1 works on my 2.3
> device (HTC Desire Z). I didn’t notice any issue on my device. 
> When the onStop method is called, the layerView is set to VISIBLE, but the
> activity is no more visible. So it’s not an issue for the accessibility.
> When you come back to the activity, if you trace the onResume method, we
> could have an accessibility issue at this point because the layerView is
> visible. But in fact there is no issue because the layerView is visible but
> is totally white. The last web page is not visible on the layerView.

How do you know that the layerView is white (and presumably lacking content)? I believe this will still cause overdraw issues (but, if we're just dumping white, they're not likely to be as significant as if we were drawing web content).
Flags: needinfo?(domivinc)
(In reply to Michael Comella (:mcomella) from comment #23)
> Yeah, I used the latest patch. Typically the animations between devices of
> different manufacturers are different - I wonder if the animations on my
> device just make this affect more noticeable. 

Really interesting point. I will have a look to animateLayoutChange flags and LayoutTransition calls in the firefox code. Something in the code could lock the default activity transition and the visibility change of the layerView unlocks the situation.

> 
> How do you know that the layerView is white (and presumably lacking
> content)? I believe this will still cause overdraw issues (but, if we're
> just dumping white, they're not likely to be as significant as if we were
> drawing web content).

Two different ways to debug what is happening in such situation:
- change the layout xml file in order to see a part of the hidden element (width/height/position of the different elements in the screen)
or
- play with setVisibility in a timer (for instance hide 5 s. the elements that are in the front)
Flags: needinfo?(domivinc)
Blocks: 1003123
I didn’t find any code change that could impact the animation between 2 activities. 

I found another way to fix the issue: in the method onWindowFocusChanged, when we get the focus, we can post on the UI thread a showSurface call. The visibility of the mLayerView is not changed, only the surface visibility is changed.
This hack is already used for another issue in the removeFullScreenPluginView method.

On my device (HTC), there is no visual change between the patch ..._6 and this patch ..._7. The issue is correctly fixed in both cases.
The bug 1003123 is also fixed by this patch.

Could you try on your Motorola device, to see if with this patch (..._7) you still have the same visible problem: “the activity seems to ‘reappear’ twice” ?
Attachment #8485829 - Flags: feedback?(michael.l.comella)
Comment on attachment 8485829 [details] [diff] [review]
patch1058027_7.diff

Review of attachment 8485829 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not familiar with the implications of displaying the surface view so I'd like a second opinion on the matter. Otherwise, this looks good to me. Thanks for your dilligent research!

::: mobile/android/base/BrowserApp.java
@@ +719,5 @@
> +        super.onWindowFocusChanged(hasFocus);
> +        // If Home Page is visible, the layerView surface has to be visible
> +        // to avoid a surface issue in Gingerbread phones.
> +        // We need to do this on the next iteration.
> +        // See bugs: 1058027 and 1003123

Mention that we need to do this on the next iteration to avoid deadlock, like: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp.java?rev=ceb1265224fd#901

Did you test that this is actually the case though?
Attachment #8485829 - Flags: review?(snorp)
Attachment #8485829 - Flags: feedback?(michael.l.comella)
Attachment #8485829 - Flags: feedback+
Clear in favor of r?.
Flags: needinfo?(snorp)
(In reply to Dominique Vincent [:domivinc] from comment #25)
> Could you try on your Motorola device, to see if with this patch (..._7) you
> still have the same visible problem: “the activity seems to ‘reappear’
> twice” ?

I no longer have this issue on my Motorola device - nice work! :)
(In reply to Michael Comella (:mcomella) from comment #26)
> Comment on attachment 8485829 [details] [diff] [review]
> patch1058027_7.diff
> 
> Review of attachment 8485829 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm not familiar with the implications of displaying the surface view so I'd
> like a second opinion on the matter. Otherwise, this looks good to me.
> Thanks for your dilligent research!
> 
> ::: mobile/android/base/BrowserApp.java
> @@ +719,5 @@
> > +        super.onWindowFocusChanged(hasFocus);
> > +        // If Home Page is visible, the layerView surface has to be visible
> > +        // to avoid a surface issue in Gingerbread phones.
> > +        // We need to do this on the next iteration.
> > +        // See bugs: 1058027 and 1003123
> 
> Mention that we need to do this on the next iteration to avoid deadlock,
> like:
> https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp.
> java?rev=ceb1265224fd#901
> 
> Did you test that this is actually the case though?

Yes, I tested without the post to UI thread and it doesn't work (on my HTC ...). 

I didn't find the true explaination regarding this issue. My best guess is that the surface is locked by 2 different processes: our process (Gecko) and the animation transition between 2 activities.
The change of the visibility in the next loop is one way to unlock the situation. The other way is to avoid the dead lock of the surface and change the visibility in the onstop methods (patch _6.diff).
Comment on attachment 8485829 [details] [diff] [review]
patch1058027_7.diff

Review of attachment 8485829 [details] [diff] [review]:
-----------------------------------------------------------------

I am not sure I really understand what is going on here, but the patch seems relatively harmless I guess. I do remember having a lot of weirdness with SurfaceView on 2.3.
Attachment #8485829 - Flags: review?(snorp) → review+
Comment on attachment 8485829 [details] [diff] [review]
patch1058027_7.diff

Review of attachment 8485829 [details] [diff] [review]:
-----------------------------------------------------------------

This approach seems good to me as well - nice work, Dominique!

Here's a try push for our test suite: https://tbpl.mozilla.org/?tree=Try&rev=1072986dc679

When this goes green, feel free to add the "checkin-needed" keyword [1] to get the change checked in. Don't forget to dupe this with bug 1003123 (and let me know if you don't have permissions)!

[1]: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Getting_the_patch_checked_into_the_tree
Attachment #8485829 - Flags: feedback+ → review+
Attachment #8483471 - Attachment is obsolete: true
Attachment #8483471 - Flags: review?(liuche)
try run in comment 31 looks green to me - add the keyword whenever you're ready, Dominique! Thanks again!
Flags: needinfo?(domivinc)
Flags: needinfo?(domivinc)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4bc2d5e9a06e
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 35
Dominique are you able to verify that this is fixed for you using Nightly on your device?
Flags: needinfo?(domivinc)
(In reply to Aaron Train [:aaronmt] from comment #35)
> Dominique are you able to verify that this is fixed for you using Nightly on
> your device?

Fixed on my device with Nightly 35.0a1 (2014-09-14).
Flags: needinfo?(domivinc)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.