682681 - tab.title should never be empty

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P2)

Description

[Dietrich Ayala (:dietrich)]

for about: pages, tab.title is empty. for title-less pages, tab.title should show URL (or portion of) like we do in Firefox for empty titles.

Comment 1

[Dietrich Ayala (:dietrich)]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/236

Pointer to Github pull-request

Comment 2

[Dietrich Ayala (:dietrich)]

patch uses xul tab's label instead of content document's title. this is more accurate (reflects what user actually sees on the tab), ensures we don't have to duplicate Firefox internal logic to generate tab title, and removes use of contentDocument which won't work in e10s world.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Initially, I didn't understand this bug, but then I realized what you were getting at.  Indeed, the purpose of tab.title is to provide addons with the title of the tab, not the title of the document loaded in the tab, so this makes sense.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 557618 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/236

Seems right. I'm seeing some test failures after applying the patch, however:

    console: [JavaScript Error: "...error: TEST FAILED: test-windows.testWindowTabsObject (failure)"]
    console: [JavaScript Error: "fail: Correct active tab ("Connecting\u2026" != "tab 1")"]
    console: [JavaScript Error: "fail: Correct title ("Connecting\u2026" != "tab 1")"]

Comment 5

[Dietrich Ayala (:dietrich)]

Frak, I only ran the tab tests.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Dietrich: ping; any chance you can finish this up?

Comment 7

[Dietrich Ayala (:dietrich)]

I dug into it a while back but made no headway. Don't have time to look into it right now, so de-assigning myself.

Comment 8

[Myk Melez [:myk] [@mykmelez]]

I dug into this and discovered platform bug 701591, whose fix would resolve the test failure.  Alternately, we can use the new httpd module KWierso just landed <https://github.com/mozilla/addon-sdk/blob/master/packages/api-utils/lib/httpd.js> to load non-data: URL test documents that won't trigger the platform bug.

Note that the Tab::title docs <https://addons.mozilla.org/en-US/developers/docs/sdk/1.2/packages/addon-kit/docs/tabs.html#title> should be updated when the change is made, as they currently describe the property as "the title of the page currently loaded in the tab", whereas they should say something like "the title of the tab (usually the title of the page currently loaded in the tab)".

Also, we should call this out in the release notes, as it's something of an API change, however slight, and even though the new behavior is what we intended all along.

I'll take this on and fix it one way or another.

cc:ing wbamberg so he's aware of the release notes angle.

Comment 9

[Dietrich Ayala (:dietrich)]

Myk are you still working on this?

Comment 10

[Myk Melez [:myk] [@mykmelez]]

Urk, sorry for the delayed response!  Your fix is actually fine; it's just being blocked by bug 701591.  Once that bug is fixed, your fix can be landed.

Comment 11

[Erik Vold [:erikvold] (please needinfo? me)]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/492

Pointer to Github pull-request

Comment 12

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/b4ba8253f15e05afe72eaec3685f140b751d7a0b
bug 682681 - tab.title should never be empty (use xul tab label instead of content document title, also makes e10s compat)

https://github.com/mozilla/addon-sdk/commit/8c5a93b7a97af394b1b3d0b9827e1e2b5e09d0c5
Merge branch 'master' into bug682681-tab-title

https://github.com/mozilla/addon-sdk/commit/64b01db4bcbc11fb091bbf7f48306767c23b6103
adding tests for bug 682681

https://github.com/mozilla/addon-sdk/commit/3ac1e78524643ba5c70623d612200094e3a041ce
Merge pull request #492 from erikvold/bug682681-tab-title

fix Bug 682681 tab.title should never be empty r=@gozala

Comment 13

[Alexandre Poirot [:ochameau]]

Erik, as this bug isn't closed yet, I'm using it to report a test breakage on localized versions of firefox:
  error: TEST FAILED: test-tabs.testBug682681_aboutURI (failure)
  error: fail: title of about: tab is not blank ("Nouvel onglet" != "New Tab")

https://github.com/mozilla/addon-sdk/blob/master/packages/addon-kit/tests/test-tabs.js#L42
Here "New Tab" is translated if you use a non-english version.

Comment 14

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Alexandre Poirot (:ochameau) from comment #13)
> Erik, as this bug isn't closed yet, I'm using it to report a test breakage
> on localized versions of firefox:
>   error: TEST FAILED: test-tabs.testBug682681_aboutURI (failure)
>   error: fail: title of about: tab is not blank ("Nouvel onglet" != "New
> Tab")
> 
> https://github.com/mozilla/addon-sdk/blob/master/packages/addon-kit/tests/
> test-tabs.js#L42
> Here "New Tab" is translated if you use a non-english version.

Ah that makes sense.. I'm going to make a new bug for this.

Comment 15

[Erik Vold [:erikvold] (please needinfo? me)]

https://github.com/mozilla/addon-sdk/commit/3ac1e78524643ba5c70623d612200094e3a041ce