500829 - Clicking empty space in awesome bar history list reloads current page

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aakash Desai [:aakashd]]

Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090626 Fennec/1.0b2

screencast: http://screencast.com/t/LBlu2hOtUy

Steps to Reproduce:
1. Clear your history and go to websites.
2. Click on the url bar to enable the history list.
3. tap/click on the whitespace.

Actual Result: 
Current page is reloaded.

Expected Result:
Nothing should happen.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Prevent calling goToURI if we are clicking on an empty label

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Might be nicer to set an _empty property on empty elements in invalidate() and use that instead?

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #2)
> Might be nicer to set an _empty property on empty elements in invalidate() and
> use that instead?

Probably.
But I supposed the cleaner way is to remove the 7 initial empty label elements (seems to be used only for displaying 'No results') which appears to cause the problems ?

Comment 4

[:Gavin Sharp [email: [e-mail]]]

They're there as an optimization to avoid having to dynamically add many elements at once during the first invalidate() call. It may be a useless optimization, but even without it we don't want to remove elements once we've added them, so after the first invalidate() you could still have this problem.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch V0.2

Ok. I've not see that.

Is that what you mean or do you prefer to use a setAttribute/getAttribute instead?

Comment 6

[:Gavin Sharp [email: [e-mail]]]

That looks good, but you need to reset _empty to false in the other branch, otherwise entries won't be selectable after an empty popup appears.

Always best to avoid getAttribute/setAttribute in favor of JS properties where possible, to avoid the need to go through xpconnect.

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch V0.3

(In reply to comment #6)
> That looks good, but you need to reset _empty to false in the other branch,
> otherwise entries won't be selectable after an empty popup appears.
>

Sounds obvious... :s

> Always best to avoid getAttribute/setAttribute in favor of JS properties where
> possible, to avoid the need to go through xpconnect.

Ok.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed: http://hg.mozilla.org/mobile-browser/rev/3909514e31d9

Comment 9

[Aakash Desai [:aakashd]]

verified FIXED on 

Build Id: Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2a1pre)
Gecko/20090803 Fennec/1.0a3pre