1015923 - Rocketbar only allows user to view url once

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Browser Chrome, defect)

Description

[Dale Harvey (:daleharvey)]

When I visit a page and focus the rocketbar, I get the current address highlighted, if I blur by pressing "close" that then focus rocketbar again, I get a blank input asking to "Enter Search and Address"

Comment 1

[Dale Harvey (:daleharvey)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/19656

STR: visit a url from the rocketbar, press the backdrop, focus url and the url is visible, press close then focus the url, input it empty and "Search or enter Address" is visible

The change to event handling isnt strictly necessary, however it vastly complicated debugging and leads to other bugs as attaching the events to the container meant that they are triggered whenever cancel / anything else in the rocketbar was pressed, they are now more isolated and will only be triggered when they should be (opening the task manager while typing a url shouldnt be possible etc)

I want more comprehensive integration tests for this functionality, however https://bugzilla.mozilla.org/show_bug.cgi?id=1009855 needs fixed before that, working on it now

Comment 2

[Ben Francis [:benfrancis]]

Comment on attachment 8428944 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/19656

Nice catch!

The actual fix (removing the reset type attribute) is fine, but please don't land the unrelated event listener changes.

The event listeners are intentionally on the container rather than the title element and I'm concerned that changing them to the title div will cause a regression in the ability to reliably swipe down from the top to show Rocketbar and activate the task manager (which is already pretty unreliable!).

When I test with this patch applied in Firefox with mousedown on the container and pull down the gesture never gets triggered. The situation may be better on a real touch device, but in the best case scenario you're still reducing the touch target size.

I would prefer to land without those changes and explore them in a follow-up with more thorough tests if you think it's a cleaner approach.

Comment 3

[Dale Harvey (:daleharvey)]

We should not add global events on containers and just hope their children do the right thing, yes we have an if != cancelbutton, but that doesnt stop the event bubbling, it means everything we add (like the back button), isnt only responsible for its own event behaviour, but also responsible for not breaking in the presence of the containers events. That is just going to lead to more buggy behaviour (its currently really easy to get rocketbar in an buggy unknown state, not so easy to find bugs with specific STR)

But this also needs fixed at the same time as we fix the conflict with utility tray, so landed with just the reset change.

https://github.com/mozilla-b2g/gaia/commit/0215fd6d11f7bfbf802636860373e5edd5cf41f3