[B2G][Email] Search Mail field appears twice on email search filter page when user reveals drag down search field

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bzumwalt, Unassigned)

Tracking

({regression})

unspecified
ARM
Gonk (Firefox OS)
regression

Firefox Tracking Flags

(blocking-b2g:-, firefox28 affected, b2g18 unaffected, b2g-v1.2 affected)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 8343222 [details]
Screenshot

Description:
On email search filter page the user can drag finger down results field, revealing a drag down "Search Mail" field. This causes two "Search Mail" fields to appear on screen at the same time (Primary search field and drag down search field.)


Repro Steps:
1) Updated Buri to Build ID: 20131205004003
2) Open Email app (with account already set up)
3) Press search icon to open email search filter
4) Drag finger down in results field to reveal second search bar

Actual:
Two search bars for same content appear on email search filter page.

Expected:
Only one search bar visible on email search filter screen.

Environmental Variables
Device: Buri v 1.2 COM RIL
Build ID: 20131205004003
Gecko: http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/af2c7ebb5967
Gaia: 0659f16b9790b1cf9eba4d80743fcc774d2ffe3a
Platform Version: 26.0
RIL Version: 01.02.00.019.102 
Firmware Version: V1.2_US_20131115

Notes:
Repro frequency: 3/3, 100%
See attached: screenshot
(Reporter)

Comment 1

5 years ago
Repros on Buri in 1.3 

Result: Two search bars for same content appear on email search filter page.

Environmental Variables
Device: Buri v 1.3 Mozilla RIL
Build ID: 20131205040201
Gecko: http://hg.mozilla.org/mozilla-central/rev/725c36b5de1a
Gaia: 1dd0e5c644b4c677a4e8fa02e50d52136db489d9
Platform Version: 28.0a1
Firmware Version: V1.2_US_20131115
Flags: needinfo?(epang)
(Reporter)

Comment 2

5 years ago
Does NOT reproduce on Buri in 1.1

Result: Only one search bar visible on email search filter screen.

Environmental Variables
Device: Buri v 1.1 COM RIL
Build ID: 20131204041429
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/c714e1bd607f
Gaia: 6ff3a607f873320d00cb036fa76117f6fadd010f
Platform Version: 18.1
RIL Version: 01.01.00.019.281 
Firmware Version: V1.2_US_20131115


Also tested on first 1.2 build for Buri - Issue does NOT reproduce on build ID: 20130621031231
status-b2g18: --- → unaffected
Keywords: regression
This is one of our more amusing bugs!  Good catch.  Not a visual glitch though, program logic.

The visibility of the search-tease-bar is controlled at a high level by it having the "msg-nonsearch-only" class.  Unfortunately, _hideSearchBoxByScrolling removes its 'collapsed' state in an attempt to iron out some caching difficulties and does not respect the state.

The simplest thing is just to have that method early return if we are in search mode.
Flags: needinfo?(epang)
While this is a legitimate regression, I don't think I'd hold the release for it.

Updated

5 years ago
No longer blocks: 925925

Updated

5 years ago
blocking-b2g: --- → 1.3?
blocking- for comment 4
blocking-b2g: 1.3? → -
Duplicate of this bug: 980663
Created attachment 8387314 [details] [review]
Github pull request

Pretty simple fix actually. Adding tests is more difficult. I don't want to conflict with the tests in bug 980627 though, so if that lands we can easily add tests here.

If we don't care for tests, we can get a review/land this. (Or someone can steal this)
Comment on attachment 8387314 [details] [review]
Github pull request

Hey Guys,

Flagging a few people for review here as this is a pretty bad visual glitch, and I would really hate to see us ship 1.4 with it. Any chance we could get this in quickly? Thanks!
Attachment #8387314 - Flags: review?(m)
Attachment #8387314 - Flags: review?(jrburke)
Attachment #8387314 - Flags: review?(bugmail)
Comment on attachment 8387314 [details] [review]
Github pull request

Thanks for diving into this!

This particular fix I do not think is enough, as it depends on the cached state to have set collapsed on the searchBar as part of saving. So technically, if there is no cookie cache (which happens on app updates the first time) then this fix would not help.

I think the more straightforward fix is to just use CSS to hide that toolbar when in the search mode. I did something like this in message_cards.css, after the other .msg-search-tease-bar style section:

[data-mode="search"] .msg-search-tease-bar {
  display: none;
}

and that was enough to fix it, no changes to the .js file.

In marionette/js/email.js, for tapSearchButton(), we normally use this._waitForTransitionEnd() for waiting for the card transition. However, in this case, since both cards are "message_list" it may not be enough on its own, so the explicit waitForElement on the search card is good. Perhaps just add the transition wait after the existing waitForElement:

this.client.helper
    .waitForElement(Selector.searchCard);
this._waitForTransitionEnd('message_list');

The main reason to have the _waitForTransition in there is because the email Cards mechanism will eat tap events if the cards are still in transition mode. I am concerned that the waitForElement on search card could return true but the transition is still finishing up. While not an issue for this specific case, other tests that may use tapSearchButton() may need subsequent taps to work.

I am not that strong on the test internals though, just going on what we have done before. If you have knowledge to the contrary though, it is probably more current than mine.

If you want to do those changes and confirm the test still works, you can flip review back to me and I can turn around review pretty quickly, during the times I'm online.
Attachment #8387314 - Flags: review?(m)
Attachment #8387314 - Flags: review?(jrburke)
Attachment #8387314 - Flags: review?(bugmail)
Comment on attachment 8387314 [details] [review]
Github pull request

Hey James! Nice fix - seems more straightforward. I've also updated the test to waitForTransition. Please take another look when you have time. Thanks!
Attachment #8387314 - Flags: review?(jrburke)
Comment on attachment 8387314 [details] [review]
Github pull request

Great, thanks!
Attachment #8387314 - Flags: review?(jrburke) → review+
Thanks, landed: https://github.com/mozilla-b2g/gaia/commit/8f802237927c7d5e024fb7dca054dd5efef6b2e6
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.