421953 - Implement RESULTS_AS_DATE_HOST_SITE_QUERY

Status: RESOLVED INACTIVE

(Firefox :: Bookmarks & History, enhancement)

Description

[Ronny Perinke]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008031004 Minefield/3.0b5pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008031004 Minefield/3.0b5pre

Bug 385245 landed a while ago and many things which we where able to do with places are not working anymore. Hence, I'm requesting a DateHostSiteQuery type (smiliar to DateQuery  and DateSiteQuery), which sort/order the resulst in the following manner:
1st level: date
2nd level: host
3rd level: site

e.g.
today > mozillazine.org > forums.mozillazine.org

this was equal to places query place:group=0&group=2&group=1&sort=1


p.s.
quote from Ondrej Brablc:
"This should be easy to implement and fast to execute. Please file a bug and try to get votes for it."

Reproducible: Always

Comment 1

[Ronny Perinke]

DateSiteQuery stands for RESULTS_AS_DATE_SITE_QUERY and DateQuery for RESULTS_AS_DATE_QUERY

Comment 2

[Ronny Perinke]

Attachment: implementation

- allows Domain- and Date-Domain-Site-Queries
- code is similar to the already existing Date-Site- and Site-Queries

Comment 3

[Ronny Perinke]

Attachment: updated patch (diff against latest trunk)

updated version of the patch diff'ed against todays trunk

Comment 4

[Dietrich Ayala (:dietrich)]

Comment on attachment 319427 [details] [diff] [review]
updated patch (diff against latest trunk)

great, thanks for doing this! however, there's no chance of getting this in at this late stage w/o unit tests. can you please add tests for this result type, and confirm that your change does not regress any of the existing tests?

Comment 5

[Ronny Perinke]

Whew!
You mean like one of those http://lxr.mozilla.org/mozilla/source/browser/components/places/tests/unit/?

Do you have some hints for me about what/how to test? :>


best,
Ronny

Comment 6

[Ondrej Brablc]

(In reply to comment #5)
> Whew!
> You mean like one of those
> http://lxr.mozilla.org/mozilla/source/browser/components/places/tests/unit/?
> 
> Do you have some hints for me about what/how to test? :>

I think you should extend this test:

http://mxr.mozilla.org/mozilla/source/toolkit/components/places/tests/unit/test_history_sidebar.js

Do you really need to use mozStorageFunctionGetDomainFromUrl, cannot you just extend existing code which works with rev_host? I did not analyze what should be faster, but you have probably done that.

Comment 7

[cmtalbert]

(In reply to comment #5)
> Whew!
> You mean like one of those
> http://lxr.mozilla.org/mozilla/source/browser/components/places/tests/unit/?
> 
> Do you have some hints for me about what/how to test? :>
It's also good to be sure to test that LiveUpdate of a result set from your query type work properly.  You can see the specific query API tests that I've done [1] (which are probably more complicated than what you want to do in a unit test) and I've got a stub test [2] to illustrate the approach for Live Update testing.
 
[1]: http://wiki.mozilla.org/QA/TDAI/Projects/Places_Tests
[2]: http://mxr.mozilla.org/mozilla/source/toolkit/components/places/tests/queries/stub-test.js

Comment 8

[Ronny Perinke]

(In reply to comment #6)
> Do you really need to use mozStorageFunctionGetDomainFromUrl, cannot you just
> extend existing code which works with rev_host? I did not analyze what should
> be faster, but you have probably done that.
> 

I wanted a reliable existing method to get the host out of the url (e.g. mozilla.org from bugzilla.mozilla.org). I chose the nsIEffectiveTLDService and because it required a valid nsIURI, I took the url instead of rev_host (the full url can be transfered easily into a nsIURI).

Thanks for the hints about the tests. I'll try to create one. ^^

Comment 9

[Ronny Perinke]

Attachment: unit test (updated history-sidebar-test)

I followed Ondrej's suggestion and extended the test for the history sidebar at http://mxr.mozilla.org/mozilla/source/toolkit/components/places/tests/unit/test_history_sidebar.js

The test is similar to the RESULTS_AS_DATE_SITE_QUERY test. I expected the following structure of the history when using RESULTS_AS_DATE_DOMAIN_SITE_QUERY:

Today
  |--google.com
    |--mirror0.google.com
      |--http://mirror0.google.com/c
      |--http://mirror0.google.com/d
  |--mozilla.com
    |--mirror0.mozilla.com
      |--http://mirror0.mozilla.com/c
      |--http://mirror0.mozilla.com/d
Yesterday
  |--google.com
    |--mirror1.google.com
      |--http://mirror1.google.com/c
      |--http://mirror1.google.com/d
  |--mozilla.com
    |--mirror1.mozilla.com
      |--http://mirror1.mozilla.com/c
      |--http://mirror1.mozilla.com/d
2 days ago
  |--google.com
    |--mirror2.google.com
      |--http://mirror2.google.com/c
      |--http://mirror2.google.com/d
  |--mozilla.com
    |--mirror2.mozilla.com
      |--http://mirror2.mozilla.com/c
      |--http://mirror2.mozilla.com/d
3 days ago
  |--google.com
    |--mirror3.google.com
      |--http://mirror3.google.com/c
      |--http://mirror3.google.com/d
  |--mozilla.com
    |--mirror3.mozilla.com
      |--http://mirror3.mozilla.com/c
      |--http://mirror3.mozilla.com/d
4 days ago
  |--google.com
    |--mirror4.google.com
      |--http://mirror4.google.com/a
      |--http://mirror4.google.com/b
  |--mozilla.com
    |--mirror4.mozilla.com
      |--http://mirror4.mozilla.com/a
      |--http://mirror4.mozilla.com/b
5 days ago
  |--google.com
    |--mirror5.google.com
      |--http://mirror5.google.com/a
      |--http://mirror5.google.com/b
  |--mozilla.com
    |--mirror5.mozilla.com
      |--http://mirror5.mozilla.com/a
      |--http://mirror5.mozilla.com/b
6 days ago
  |--google.com
    |--mirror6.google.com
      |--http://mirror6.google.com/a
      |--http://mirror6.google.com/b
  |--mozilla.com
    |--mirror6.mozilla.com
      |--http://mirror6.mozilla.com/a
      |--http://mirror6.mozilla.com/b
Older than 6 days
  |--google.com
    |--mirror7.google.com
      |--http://mirror7.google.com/a
      |--http://mirror7.google.com/b
  |--mozilla.com
    |--mirror7.mozilla.com
      |--http://mirror7.mozilla.com/a
      |--http://mirror7.mozilla.com/b

The leafs of each node are visits.
I tested "4 days ago" for the expected structure and it passed the test.

Here is the result:
*** test pending
*** test finished
*** exiting
*** PASS ***
Exception: [Exception... "Component returned failure code: 0x80520015 (NS_ERROR_FILE_ACCESS_DENIED) [nsIFile.remove]"  nsresult: "0x80520015 (NS_ERROR_FILE_ACCESS_DENIED)"  location: "JS frame :: ../../../../_tests/xpcshell-simple/test_places/unit/head_bookmarks.js :: clearDB :: line 99"  data: no]

Unfortunately I do not have an explanation for the exception, because "everyone" has all permissions and it's not write-protected.

Comment 10

[Ronny Perinke]

Attachment: updated patch

- includes sidebar unit test

- additions to browser/components/places/content/controller.js in _buildSelectionMetadata() used by buildContextMenu()

- adds nodeIsDomain() to PlacesUtils and updates nodeIsDay() as well as nodeIsHistoryContainer()


Would be nice to have it in Firefox 3.1 ;-)

Comment 11

[Gervase Markham [:gerv]]

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

Comment 12

[Dietrich Ayala (:dietrich)]

Comment on attachment 328016 [details] [diff] [review]
updated patch

Cleaning out my review queue. Wow, I am sad to have let this sit so long. I'm very sorry Ronny. This is likely rotted quite far at this point.

Comment 13

[BMO Automation]

Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.