If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

VERIFIED FIXED in Firefox 17

Status

()

Firefox for Android
General
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: wesj, Assigned: wesj)

Tracking

unspecified
Firefox 17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

5 years ago
Created attachment 645338 [details] [diff] [review]
Patch

We should fall back to the url in this case?
(Assignee)

Updated

5 years ago
Attachment #645338 - Flags: review?(margaret.leibovic)

Comment 1

5 years ago
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.
Attachment #645338 - Attachment is patch: true
Attachment #645338 - Flags: review?(margaret.leibovic) → review-
(Assignee)

Comment 2

5 years ago
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()?
(Assignee)

Comment 3

5 years ago
Created attachment 645494 [details] [diff] [review]
Patch v2

?
Assignee: nobody → wjohnston
Attachment #645338 - Attachment is obsolete: true
Attachment #645494 - Flags: review?(margaret.leibovic)

Comment 4

5 years ago
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.
Attachment #645494 - Flags: review?(margaret.leibovic) → review+
(Assignee)

Comment 5

5 years ago
Created attachment 645585 [details] [diff] [review]
Patch 2/2

Good idea. Cleans up the tabs stuff.
Attachment #645585 - Flags: review?(margaret.leibovic)

Comment 6

5 years ago
(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...
(Assignee)

Comment 7

5 years ago
Created attachment 645592 [details] [diff] [review]
Patch 2/2 v2.

Grr.
Attachment #645585 - Attachment is obsolete: true
Attachment #645585 - Flags: review?(margaret.leibovic)
Attachment #645592 - Flags: review?(margaret.leibovic)

Updated

5 years ago
Attachment #645592 - Flags: review?(margaret.leibovic) → review+
(Assignee)

Comment 8

5 years ago
Part 1: https://hg.mozilla.org/integration/mozilla-inbound/rev/1dd57c1c57cf
(Assignee)

Comment 9

5 years ago
Part 2: https://hg.mozilla.org/integration/mozilla-inbound/rev/4f5722507e68
https://hg.mozilla.org/mozilla-central/rev/1dd57c1c57cf
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17

Comment 11

5 years ago
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.