Closed Bug 593905 Opened 10 years ago Closed 10 years ago

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

Categories

(Firefox Graveyard :: Panorama, defect, P3)

defect

Tracking

(Not tracked)

VERIFIED FIXED
Future

People

(Reporter: raymondlee, Assigned: mgway12)

References

Details

Attachments

(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 (tab.tab != undefined) 
      tab = tab.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:

/browser/base/content/test/tabview/browser_tabview_search.js
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.

a=beltzner
Attachment #491102 - Flags: approval2.0? → approval2.0+
http://hg.mozilla.org/mozilla-central/rev/ef059bdf7be3
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
verified in current minefield nightly.
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.