Closed
Bug 1120530
Opened 10 years ago
Closed 10 years ago
Reproducible 'waiting for urlbar text to gain focus' when first run experience is visible
Categories
(Firefox for Android Graveyard :: Testing, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 38
People
(Reporter: rnewman, Assigned: rnewman)
References
()
Details
(Keywords: reproducible)
Attachments
(1 file)
(deleted),
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
* Clear data.
* Run a Robocop test that waits for URL bar focus -- e.g., testClearPrivateData.
* Observe that the screen stays stuck on the FRE, and the test fails:
Ran 4 tests (1 parents, 3 subtests)
Expected results: 2
Unexpected results: 2 (FAIL: 2)
Unexpected Results
==================
testClearPrivateData
--------------------
FAIL waiting for urlbar text to gain focus
FAIL Exception caught
with these in the log:
01-12 10:27:43.801 D/Telemetry( 4267): SendUIEvent: event = cancel.1 method = dialog timestamp = 989904 extras = firstrun-pane
01-12 10:27:53.954 I/Robocop ( 4267): {"thread":null,"source":"robocop","pid":null,"message":"waitForCondition timeout after 10000 ms.","time":1421087273949,"action":"log","level":"info"}
01-12 10:27:53.960 I/Robocop ( 4267): {"thread":null,"source":"robocop","pid":null,"status":"FAIL","time":1421087273954,"expected":"PASS","action":"test_status","test":"testClearPrivateData","subtest":"waiting for urlbar text to gain focus","message":"urlbar text gained focus"}
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 1•10 years ago
|
||
This is the idea I had way back.
Attachment #8547709 -
Flags: review?(mark.finkle)
Comment 2•10 years ago
|
||
Comment on attachment 8547709 [details] [diff] [review]
Don't show first run experience in Robocop tests. v1
Review of attachment 8547709 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/base/tests/BaseTest.java
@@ +114,5 @@
> Intent i = new Intent(Intent.ACTION_MAIN);
> mProfile = mConfig.get("profile");
> +
> + // Don't show the first run experience.
> + i.putExtra(BrowserApp.EXTRA_SKIP_START_PANE, true);
You probably want to do this in UITest too (those tests don't inherit from BaseTest).
Comment 3•10 years ago
|
||
Comment on attachment 8547709 [details] [diff] [review]
Don't show first run experience in Robocop tests. v1
>diff --git a/mobile/android/base/BrowserApp.java b/mobile/android/base/BrowserApp.java
>+ public static final String EXTRA_SKIP_START_PANE = "skipstartpane";
EXTRA_SKIP_START_PANE -> EXTRA_SKIP_STARTPANE
>- * Check and show Onboarding start pane if Firefox has never been launched and
>+ * Check and show onboarding start pane if Fennec has never been launched and
s/Firefox|Fennec/browser
Address Geoff's comment too!
Attachment #8547709 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Comment 5•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Comment 7•10 years ago
|
||
Instead of making an extra pref, is there a reason why you don't just use PREF_START_PANE_ENABLED and pass false into the intent?
Updated•10 years ago
|
Flags: needinfo?(rnewman)
Assignee | ||
Comment 8•10 years ago
|
||
(In reply to Chenxia Liu [:liuche] from comment #7)
> Instead of making an extra pref, is there a reason why you don't just use
> PREF_START_PANE_ENABLED and pass false into the intent?
I'm not sure I understand your question. This patch didn't make a pref, and the P_S_P_E approach doesn't use the intent.
(Did you mean "intent extra" instead of "extra pref"?)
The code prior to the patch checks the value of a per-profile shared pref to decide whether to show the start pane. After showing the pane, it sets that pref so it isn't shown again.
There was no intent-based aspect to this at all: to use P_S_P_E from tests we'd have to find the profile, open SharedPreferences, and set the pref before launching the activity, and that will persist.
The approach I took was to set an extra on the intent itself. No read, no write; whether to show the start pane is a property of the intent itself.
Flags: needinfo?(rnewman)
Comment 9•10 years ago
|
||
No, you're right - I was stuck thinking about prefs and thought Intents could take prefs or something, which is pretty bogus now that I stop to think about it.
Passing an intent extra seems odd - isn't there a way to mock some SharedPreferences values for tests?
Assignee | ||
Comment 10•10 years ago
|
||
We definitely want to do that, partly to solve our tests running against the persistent app prefs (touched on in Bug 1069687). But mocking all of our use of SharedPreferences is a much bigger change, so I wanted to fix the bug first.
The intent extra approach isn't that odd; it's basically how you pass arguments via intents, so this is only as awful as any other boolean flag.
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•