places default title behavior shouldn't use part of the URL

RESOLVED FIXED in mozilla1.9.3a1

Status

()

defect
--
major
RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: aakashd, Assigned: mak)

Tracking

(Depends on 1 bug)

Trunk
mozilla1.9.3a1
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
in-litmus ?

Firefox Tracking Flags

(fennec1.0+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

10 years ago
Build ID:
Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20090928
Fennec/1.0b4pre

Litmus Testcase https://litmus.mozilla.org/show_test.cgi?id=7612

Steps to Reproduce:
1. Go to http://ed.agadak.net/blan
2. Go to the awesome bar and view the page title listed there

Actual Results:
"blank" shows up in the page title and not the url

Expected Results:
Due to the patch in https://bugzilla.mozilla.org/show_bug.cgi?id=456084 , the url should show up in the title.
Reporter

Updated

10 years ago
Severity: normal → major
tracking-fennec: --- → ?
Reporter

Comment 1

10 years ago
This is also inconsistent with the bookmarks manager as the name field in the edit bookmark dialog shows the title to be the url there.
bug 456084 only fixed blanks for the URLBar itself. A different bug fixed blanks in bookmarks.
"blank" seems to be coming from the autocomplete controller, which kinda means that we are somehow saving it as "blank" - but Firefox doesn't
Turns out this is places' intended behavior. It tries to generate a title based on the URL for history entries that have no title:

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/src/nsNavHistory.cpp#7425 (GenerateTitleFromURI)

This happens in Firefox as well. We could turn this into a bug asking for that behavior to be changed, but otherwise I think it is INVALID.
(That page just happens to be "blank" because that's the last part of its URL)
(In reply to comment #5)
> (That page just happens to be "blank" because that's the last part of its URL)

So it's working as designed. I think we should close this bug too.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → INVALID
Reporter

Comment 7

10 years ago
There's an inconsistency to the user for the bookmarks dialog to have a different page title than the one in the awesome bar. This is definitely a bug.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Not a Fennec bug, though...
Component: General → Places
Product: Fennec → Toolkit
QA Contact: general → places
Summary: Pages with blank titles show "blank" in the awesome bar results → places default title behavior shouldn't use part of the URL
Actually saving null titles is probably the best option - the consumer can decide what to actually show (e.g. by duplicating GenerateTitleFromURI in front-end code).

Note also that there is an inconsistency in behavior, which probably explains why we sometimes didn't see this bug in testing - InternalAddNewPage calls GenerateTitleFromURI if the title is void, but setPageTitle() called from nsDocShell::SetTitle doesn't, and seems to just save null titles.
Assignee

Comment 10

10 years ago
i completely agree with Gavin as i said or IRC, toolkit should not make assumption on titles, and leave decisions on how to fill empty titles to the frontend.
Saving a part of the uri as title means deciding that's the best option, and as we see here, is it not always true.
Saving meaningless informations is always bad from a db point of view, and from a user point of view.
We should really get rid of that code and just save a null title.
Reporter

Comment 11

10 years ago
This'll make changes to litmus test case on the fennec 1.0 test run:

https://litmus.mozilla.org/show_test.cgi?id=7612
Flags: in-litmus?
Assignee

Updated

10 years ago
Assignee: nobody → mak77
Status: REOPENED → ASSIGNED
Assignee

Updated

10 years ago
Flags: in-testsuite?
Assignee

Comment 12

10 years ago
the interesting question is:
is an empty string title different from a null title?

or in other words: should returned title be empty string or null, or should we always return an empty string (for example from getPageTitle)?

From a code point of view, since we allow to set a null title or an empty string title makes sense to allow distinguish them. But from a "contents" point of view this is the title of a page, and the page either has a title or has not.

I think i'll go for the widest impl, allowing to set and get both an empty string or a null title, so that an implementer can choice to only override null titles while setting empty string titles for some wanted page.
Assignee

Comment 13

10 years ago
Posted patch patch v1.0 (obsolete) — Splinter Review
Assignee

Comment 14

10 years ago
Posted patch patch v1.1Splinter Review
Attachment #406439 - Attachment is obsolete: true
Assignee

Updated

10 years ago
Attachment #406441 - Flags: review?(dietrich)

Updated

10 years ago
tracking-fennec: ? → 1.0+
Attachment #406441 - Flags: review?(dietrich) → review+
Assignee

Comment 15

10 years ago
http://hg.mozilla.org/mozilla-central/rev/86f3d9c54c95
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago10 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Assignee

Comment 16

10 years ago
pushed an additional changeset to fix an orange: http://hg.mozilla.org/mozilla-central/rev/dcfa9c3a2388
Assignee

Comment 17

10 years ago
Comment on attachment 406441 [details] [diff] [review]
patch v1.1

this is blocking fennec, would probably need approval
Attachment #406441 - Flags: approval1.9.2?
Duplicate of this bug: 535801
Attachment #406441 - Flags: approval1.9.2? → approval1.9.2.2?
Comment on attachment 406441 [details] [diff] [review]
patch v1.1

Minusing this version because 1) we missed 1.9.2.2 and 2) this patch apparently needs a test fix before landing.

Does this really block Fennec? Because it doesn't seem to have... Anyway, if you still need this please create a merged 1.9.2.x patch with text fix and request 1.9.2.3 approval
Attachment #406441 - Flags: approval1.9.2.2? → approval1.9.2.2-
Assignee

Comment 20

9 years ago
(In reply to comment #19)
> Does this really block Fennec?

not blocking in the strict sense, but pretty annoying unless they did workaround it (empty selection from the locationbar), that they should tell us.
Fennec didn't work around it, AFAIK. It's makes awesomebar results somewhat annoying for pages without titles, but it's certainly not the end of the world.
Depends on: 1233414
You need to log in before you can comment on or make changes to this bug.