Closed Bug 593905 Opened 10 years ago Closed 10 years ago

Add the ability to search the URLs of tabs in Panorama UI


(Firefox Graveyard :: Panorama, defect, P3)



(Not tracked)



(Reporter: raymondlee, Assigned: mgway12)




(1 file, 1 obsolete file)

We only search for titles of tabs, we should supportsearching on tabs' urls.
Depends on: 592045
Priority: -- → P3
Hardware: x86 → All
While definitely useful and desired, I think we can safely bump this to post Firefox 4 land.
Target Milestone: --- → Future
Match against tab.url as well as tab.label. No priority in terms of "best match" is given to either match type.
Attachment #488944 - Flags: feedback?(ian)
Comment on attachment 488944 [details] [diff] [review]
Match against tab.url as well as tab.label

Matthew, thank you for doing this! Comments: 

* Looks like you're using tabs instead of spaces. Please reformat with spaces; two per indent.

>+  URLOf: function TabUtils_URLOf(tab) {
>+	return tab.url != undefined ? tab.url : tabItem.url;
>+  },

The TabUtils routines have been written to try the xul:tab access first; might as well continue that here. As far as I know, xul:tabs don't have a .url property; TabItems do, though not always... might as well just ignore it and always take it straight from the tab. I propose: 

  URLOf: function TabUtils_URLOf(tab) {
    // Convert a <TabItem> to a <xul:tab>
    if ( != undefined) 
      tab =;
    return tab.linkedBrowser.currentURI.spec;

>+	  var url = TabUtils.URLOf(tab);

I know there are a bunch of vars in this code, but we're trying to move to using "let" instead, at least for new code. 

Seems like this patch needs a test... can probably just update the existing search test:

Attachment #488944 - Flags: feedback?(ian) → feedback-
Assignee: nobody → mgway12
Not sure about the accuracy of the tests, I haven't written any tests for mochitest before.
Attachment #488944 - Attachment is obsolete: true
Comment on attachment 491102 [details] [diff] [review]
v2. Fixed whitespace issues, used let instead of var, added tests.

Looks great!
Attachment #491102 - Flags: review+
Attachment #491102 - Flags: approval2.0?
Comment on attachment 491102 [details] [diff] [review]
v2. Fixed whitespace issues, used let instead of var, added tests.

Attachment #491102 - Flags: approval2.0? → approval2.0+
Closed: 10 years ago
Resolution: --- → FIXED
verified in current minefield nightly.
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.