776950 - Long tap on titlebar can't create homescreen shortcut if page has no title

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Wesley Johnston (:wesj)]

Attachment: Patch

We should fall back to the url in this case?

Comment 1

[:Margaret Leibovic]

Comment on attachment 645338 [details] [diff] [review]
Patch

Wht is tab.getTitle() returning null? I think a better fix would be to make sure that doesn't happen in Tab. This method and shareCurrentURI are the only consumers of tab.getTitle, and I think both of them would like to default to the URL as a title, so let's put the change in there.

Comment 2

[Wesley Johnston (:wesj)]

I think we want getTitle to return null if there is none. That gives us some way to know that the tab doesn't have a title.

How about I just switch both to using tab.getDisplayTitle()?

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch v2

?

Comment 4

[:Margaret Leibovic]

Comment on attachment 645494 [details] [diff] [review]
Patch v2

That sounds good, considering we already have a method for that :)

It looks like the only place getTitle() is used then is instead Tab.java itself:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/Tab.java#422

Bonus points if you write a patch that kills getTitle and replaces that call with mTitle. We don't need the misdirection of a getter method inside the class itself, and if that method isn't used anymore we should kill it. We can always add it back if we need it for something.

Comment 5

[Wesley Johnston (:wesj)]

Attachment: Patch 2/2

Good idea. Cleans up the tabs stuff.

Comment 6

[:Margaret Leibovic]

(In reply to Wesley Johnston (:wesj) from comment #5)
> Created attachment 645585 [details] [diff] [review]
> Patch 2/2
> 
> Good idea. Cleans up the tabs stuff.

This patch is empty...

Comment 7

[Wesley Johnston (:wesj)]

Attachment: Patch 2/2 v2.

Grr.

Comment 8

[Wesley Johnston (:wesj)]

Part 1: https://hg.mozilla.org/integration/mozilla-inbound/rev/1dd57c1c57cf

Comment 9

[Wesley Johnston (:wesj)]

Part 2: https://hg.mozilla.org/integration/mozilla-inbound/rev/4f5722507e68

Comment 10

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/1dd57c1c57cf

Comment 11

[Ioana Chiorean]

Build ID: 17.0 (2012-10-08) Aurora Channel
          18.0 (2012-10-08) Nightly Channel
          19.0 (2012-10-09) Nightly Channel
Device: Samsung Galaxy Nexus
OS: Android 4.1

Tried with about:home and the page is added to home screen. 
Marking bug as Verify Fixed.