1452621 - Cleanup some tag queries related code

Status: RESOLVED FIXED

(Toolkit :: Places, enhancement, P2)

Description

[Marco Bonardo [:mak]]

I found a few code lines that could be simplified considered the direction we are taking.
For example, most of the exceptions in serializeNode are not really that useful.

Comment 1

[Marco Bonardo [:mak]]

Attachment: Bug 1452621 - Cleanup some tag queries related code.

Review commit: https://reviewboard.mozilla.org/r/234976/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/234976/

Comment 2

[Marco Bonardo [:mak]]

hm looks like nodeIsBookmark() returns true for some virtual query, that's surprising, or maybe not, looks like we return a 0 itemId, instead of -1 for virtual entries?

Comment 3

[Marco Bonardo [:mak]]

So what actually happens is that with my change Properties would be disabled on the roots, because the virtual roots have a concreteId but not an id. It may even be ok, there's no much point in opening the read-only properties dialog of a root.
Though, I must figure out what to do of the mochitest-browser that checks first if Properties is enabled. Probably the test should use the Library pane instead to check readonly.

Comment 4

[Marco Bonardo [:mak]]

Comment on attachment 8966222 [details]
Bug 1452621 - Cleanup some tag queries related code.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/234976/diff/1-2/

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 8966222 [details]
Bug 1452621 - Cleanup some tag queries related code.

https://reviewboard.mozilla.org/r/234976/#review240980

Yay, especially nice to get rid of the grandParentId code, and to simplify serializeNode in general.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b9d9f133bee
Cleanup some tag queries related code. r=standard8

Comment 7

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/5b9d9f133bee