Closed Bug 430659 Opened 14 years ago Closed 14 years ago

Saved search in sidebar does not work.

Categories

(Firefox :: Bookmarks & History, defect, P2)

defect

Tracking

()

VERIFIED FIXED
Firefox 3

People

(Reporter: alice0775, Assigned: dietrich)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-us; rv:1.9pre) Gecko/2008042402 Minefield/3.0pre Firefox/2.0.0.14
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-us; rv:1.9pre) Gecko/2008042402 Minefield/3.0pre

Saved search(saved query) in sidebar does not work.

Reproducible: Always

Steps to Reproduce:
1.Create saved search(saved query)
2.Open sidebar
3.Crick the saved search
Actual Results:  
Nothing result.

Expected Results:  
Show search result.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042400 Minefield/3.0pre

Does indeed nothing. I don't know if this is intended.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
This should show the search results
Flags: blocking-firefox3?
I bet we just forgot to tell the sidebar that these things are folders.
Flags: blocking-firefox3? → blocking-firefox3+
Assignee: nobody → mak77
there was a previous fix for this IIRC so could have regressed recently? Can someone take a look if this is a recent or an old one?
Keywords: qawanted
The search broke on 19 Nov 2007: http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1195521720&maxdate=1195524719
The next day, 20 Nov 2007, the search was fixed but the new saved searches did not expand anymore in the sidebar. But the old saved searches (from before the range) are still expanding, even now.
I don't know if it was repaired later, couldn't find it with random checks.
When I create New Search, Library generate a query with expandQueries=0.

I think it must be expandQueries=1.
Is still any QA work needed on this bug? It looks like a regression from bug 387746?
Keywords: regression
OS: Windows XP → All
why are we doing this in treeView.js

  isContainer: function PTV_isContainer(aRow) {
    this._ensureValidRow(aRow);

    var node = this._visibleElements[aRow].node;
    if (PlacesUtils.nodeIsContainer(node)) {
      // the root node is always expandable
      if (!node.parent)
        return true;

      // treat non-expandable queries as non-containers
      if (PlacesUtils.nodeIsQuery(node)) {
        asQuery(node);
        return node.queryOptions.expandQueries;
      }
      return true;
    }
    return false;
  },

should not expandQueries be referred to children of the query instead of to the query itself?
would not be more correct doing return asQuery(node.parent).queryOptions.expandQueries;?
This way treeView.isContainer returns false in the sidebar, so the query does not expand, and i also cannot middle-click it to open contents (since we use isContainer to check if middle-click is enabled).
(In reply to comment #6)
> When I create New Search, Library generate a query with expandQueries=0.
> 
> I think it must be expandQueries=1.
> 

expandQueries=1 means that any *children* of the container that are queries should not be expanded.

we don't want place: URI bookmarks in search results and query contents to be shown as containers. (considering they're containers, maybe we shouldn't show them at all...)

(In reply to comment #8)
> 
> should not expandQueries be referred to children of the query instead of to the
> query itself?

yes, this is correct.

> would not be more correct doing return
> asQuery(node.parent).queryOptions.expandQueries;?

yes.
Assignee: mano → dietrich
Attached patch fix, per comment #8 (obsolete) — Splinter Review
with this, queries are properly expandable in the sidebar, and not so in the library.
Attachment #318686 - Flags: review?(mconnor)
Priority: -- → P2
Whiteboard: [has patch][needs review mconnor]
Target Milestone: --- → Firefox 3
Comment on attachment 318686 [details] [diff] [review]
fix, per comment #8

you need to check if node.parent is a query first.
Attachment #318686 - Flags: review?(mconnor) → review-
Attachment #318686 - Attachment is obsolete: true
Attachment #318690 - Flags: review?(mano)
Whiteboard: [has patch][needs review mconnor] → [has patch][needs review mano]
Comment on attachment 318690 [details] [diff] [review]
v2 - comment fixed

null-check parent.

r=mano otherwise.
Attachment #318690 - Flags: review?(mano) → review+
Whiteboard: [has patch][needs review mano] → [has patch][has reviews]
Comment on attachment 318690 [details] [diff] [review]
v2 - comment fixed

drivers: low impact fix
Attachment #318690 - Flags: approval1.9?
Whiteboard: [has patch][has reviews] → [has patch][has reviews][needs approval]
Comment on attachment 318690 [details] [diff] [review]
v2 - comment fixed

a1.9+=damons
Attachment #318690 - Flags: approval1.9? → approval1.9+
Checking in browser/components/places/content/treeView.js;
/cvsroot/mozilla/browser/components/places/content/treeView.js,v  <--  treeView.js
new revision: 1.68; previous revision: 1.67
done
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews][needs approval]
Keywords: qawanted
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050104 Minefield/3.0pre as well as the Win Vista nightly.
Status: RESOLVED → VERIFIED
Flags: in-litmus?
Test case https://litmus.mozilla.org/show_test.cgi?id=7506 was created on the litmus 3.0 test run for regression testing this bug.
Flags: in-litmus? → in-litmus+
Test case https://litmus.mozilla.org/show_test.cgi?id=7507 was created on the
litmus 3.1 test run for regression testing this bug.
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.