[Browser] Top Sites thumbnails showing settings screens

VERIFIED FIXED in Firefox OS v2.2

Status

Firefox OS
Gaia::System::Browser Chrome
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: Hermes Cheng (inactive after July 27, 2015), Assigned: daleharvey)

Tracking

({regression})

unspecified
2.2 S9 (3apr)
ARM
Gonk (Firefox OS)
regression

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.1 unaffected, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: [systemsfe] , URL)

Attachments

(5 attachments)

+++ This bug was initially created as a clone of Bug #1131268 +++

Because verification bug 1131268 was failed, clone that bug for following tracking.

Description:
When the user tries to go to a website without a data or Wi-Fi connection, the internet connection error screen is displayed. When the user selects "Check settings" and connect to a Wi-Fi network or data connection and return to Browser, a thumbnail shows the settings view under TOP SITES.

Pre-requisite: Have no data or Wi-Fi connected.

Repro Steps:
1) Update a Flame to 20150313010238.
2) Open Browser, and select one of the default top sites (ex. "Mozilla").
3) Select "Check settings", and connect to a Wi-Fi netowrk or cell data.
4) Press the home button, and open Browser.
5) Observe the thumbnails under TOP SITES.

Actual:
The settings view appears under one of the top sites with the URL selected from step 2.

Expected:
The settings view does not appear under top sites.

Environmental Variables:
Device: Flame Master (KK, 319mb, full flash)
Build ID: 20150313010238
Gaia: eabe35cf054d47087b37c1ca7db8143717fbd7f3
Gecko: 42afc7ef5ccb
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 39.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Repro frequency: 3/3
See attached: screenshot, video clip, logcat
http://youtu.be/bZQd9nxXeXk
Blocks: 1131268
No longer depends on: 1131268
Dale, any idea whats going on?
Flags: needinfo?(dale)
(Assignee)

Comment 2

3 years ago
Nope, this is a bit of a nasty one to test, looking into it now
Assignee: nobody → dale
Flags: needinfo?(dale)
(Assignee)

Comment 3

3 years ago
Confirmed still broken
(Assignee)

Comment 4

3 years ago
Alive, so this is an issue even if we get the bottommost window it will still run though getting the frontWindow code, taking a look through the code but got any ideas how to avoid this?
Flags: needinfo?(alive)
(In reply to Dale Harvey (:daleharvey) from comment #4)
> Alive, so this is an issue even if we get the bottommost window it will
> still run though getting the frontWindow code, taking a look through the
> code but got any ideas how to avoid this?

Ya, I forgot this case.
My idea would be always letting getScreenshot going to top most window, but one more arguments to prevent that in your call.
For normal case: appWindow.getScreenshot();
For your case: appWindow.getScreenshot(null, null, null, null, true);

And add an argument here:
BrowserMixin#getScreent: function(callback, width, height, timeout, useBottomMost)

How about this?
Flags: needinfo?(alive)
Created attachment 8585751 [details] [review]
[gaia] daleharvey:1146230 > mozilla-b2g:master
(Assignee)

Comment 7

3 years ago
Comment on attachment 8585751 [details] [review]
[gaia] daleharvey:1146230 > mozilla-b2g:master

Ok yeh I was hoping to avoid that but I dont think there is a particularly cleaner way.

This works great thanks a lot
Attachment #8585751 - Flags: review?(alive)
Comment on attachment 8585751 [details] [review]
[gaia] daleharvey:1146230 > mozilla-b2g:master

\O/
Attachment #8585751 - Flags: review?(alive) → review+
(Assignee)

Comment 9

3 years ago
Green @ https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=4f9d58a3e55ca051250e585d19050baa4e301880

Landed in: https://github.com/mozilla-b2g/gaia/commit/0da33f52d28f4d9062d6291c7a09a04e52fea105

Thanks loads Alive
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S9 (3apr)
This bug has been verified as "pass" on latest Nightly build of Flame v3.0 by the STR in Comment 0.

Actual results: The settings view will not appear under top sites.
See attachment: verified_v3.0.3gp
Reproduce rate: 0/6


Device: Flame v3.0 build(Verified)
Build ID               20150623010204
Gaia Revision          311c4e59936a407e64509f54fecb440d8a78e3c8
Gaia Date              2015-06-20 20:21:42
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/be81b8d6fae9
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150623.043033
Firmware Date          Tue Jun 23 04:30:44 EDT 2015
Bootloader             L1TC000118D0
status-b2g-master: --- → verified
QA Whiteboard: [MGSEI-Triage+]
Hi Josh,

    This bug still can be repro on latest Flame v2.2 by the STR of comment 0.  Please help to confirm whether it will approval to v2.2?  

Thank you very much.


-----------------------------------------------------------------------
Actual results (v2.2):   The settings view will appear under top sites.
Please see attachments:  settings_v2.2.3gp and logcat_v2.2_2145.txt.
Reproduce rate:          5/5.

Device: Flame v2.2 build(affected)
Build ID               20150619002501
Gaia Revision          1c33072e33c279c8aa5cb5e4a3e4da6af6cd818b
Gaia Date              2015-06-19 01:36:47
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/5ad34a170633
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150619.042950
Firmware Date          Fri Jun 19 04:30:02 EDT 2015
Bootloader             L1TC000118D0

Note: This bug can be repro on latest Flame v2.1.
status-b2g-v2.1: --- → unaffected
status-b2g-v2.2: --- → affected
Flags: needinfo?(jocheng)
Created attachment 8625659 [details]
logcat_v2.2_2145.txt
Sorry to note this wrong info: "Note: This bug can be repro on latest Flame v2.1." in Comment 12, it can't be reproduced on latest Flame v2.1, please ignore, thanks.

Comment 16

3 years ago
Hi Dale,
Can you help to raise 2.2 uplift request?
Thanks!
Flags: needinfo?(jocheng) → needinfo?(dale)
(Assignee)

Comment 17

3 years ago
Hey Josh

I would be much more comfortable if whoever wants to have this uplifted would do the request, I can help out with getting it landed if it is approved, but I dont have any context for justifying this uplift so it seems wrong for me to be making the request.

Cheers
Flags: needinfo?(dale) → needinfo?(jocheng)

Updated

3 years ago
blocking-b2g: --- → 2.2+
Flags: needinfo?(jocheng)

Comment 18

3 years ago
Hi Dale,
This is regression for 2.2 and I can approve the uplift request if you can raise the PR and uplift request.
Thanks!
Flags: needinfo?(dale)
This issue is Verified Fixed for Flame 2.2.

Result: Settings image no longer displaying in Top Sites.

Environmental Variables:
Device: Flame 2.2 KK Full Flash (319mb)
BuildID: 20150629162503
Gaia: b39d4f5b4937592ded19ec65e113a74177ae1f86
Gecko: cefa70ef71e4
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.5) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+] → [QAnalyst-Triage?][MGSEI-Triage+]
status-b2g-v2.2: affected → verified
Flags: needinfo?(ktucker)
The video for this bug is currently showing a different issue than the text of this bug since this bug was clone over to Bug 1131268. Please ignore comment 1.
status-b2g-v2.2: verified → affected
QA Whiteboard: [QAnalyst-Triage?][MGSEI-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
Flags: needinfo?(ktucker)
(Assignee)

Comment 21

3 years ago
Comment on attachment 8585751 [details] [review]
[gaia] daleharvey:1146230 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined: Minimal
[Testing completed]: Unit tests added and baked on master
[Risk to taking this patch] (and alternatives if risky): 
[String changes made]: None
Flags: needinfo?(dale)
Attachment #8585751 - Flags: approval-gaia-v2.2?
(Assignee)

Comment 22

3 years ago
> This is regression for 2.2 and I can approve the uplift request if you can raise the PR and uplift request.

I know its a regression, but I dont know what the state of the 2.2 release is plus this is an extremely minor issue, the approval system is specifically to avoid the same person asking for uplift as can approve it and I dont particularly think this needs uplifted. made the approval request for now but in future would prefer if someone wants approval, they ask for it, not have someone else ask on their behalf

Cheers
Dale

Comment 23

3 years ago
Comment on attachment 8585751 [details] [review]
[gaia] daleharvey:1146230 > mozilla-b2g:master

Noted and thanks, Dale.
Attachment #8585751 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Keywords: verifyme
This bug has been verified as "pass" on latest Nightly build of Flame v2.2 by the STR in Comment 0.

Actual results: The settings view does not appear under top sites.
See attachment: verified_Flame_v2.2.3gp
Reproduce rate: 0/10


Device: Flame v2.2 build(Verified)
Build ID               20150709162504
Gaia Revision          84d0c76370dcd3d25813b00de55194730884355b
Gaia Date              2015-07-09 13:09:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e002005dc994
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150709.202124
Firmware Date          Thu Jul  9 20:21:35 EDT 2015
Bootloader             L1TC000118D0
status-b2g-v2.2: fixed → verified
Keywords: verifyme
Created attachment 8631948 [details]
verified_Flame_v2.2.3gp
You need to log in before you can comment on or make changes to this bug.