Closed Bug 1428342 Opened 2 years ago Closed 2 years ago

Make Places queries directly inherit options from their parent

Categories

(Toolkit :: Places, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: mak, Assigned: mak)

References

(Depends on 1 open bug)

Details

(Whiteboard: [fxsearch])

Attachments

(1 file)

This simplifies some of the results code and helps with virtualization of the left pane folder.
Comment on attachment 8941137 [details]
Bug 1428342 - Make Places queries directly inherit options from their parent.

https://reviewboard.mozilla.org/r/211408/#review217446

Looks good, though there's some test failures on try that need looking at.
Attachment #8941137 - Flags: review?(standard8)
The patch was breaking the Downloads view in the Library, because some code was doing apparently pointless changes...
In any case, I'd delay landing any virtualization work (including this) to 60 at this point, to stay on the safe side. In the end this patch is mostly necessary for the work in bug 1423896 that is unlikely to land during the soft freeze.
hm, some other failures on Try, different ones :\
Comment on attachment 8941137 [details]
Bug 1428342 - Make Places queries directly inherit options from their parent.

https://reviewboard.mozilla.org/r/211408/#review217844

::: toolkit/components/places/nsNavHistoryResult.h:477
(Diff revision 4)
>    // this is set to the default in the constructor.
>    bool mExpanded;
>  
>    // Filled in by the result type generator in nsNavHistory.
>    nsCOMArray<nsNavHistoryResultNode> mChildren;
> -
> +  nsCOMPtr<nsNavHistoryQueryOptions> mOriginalOptions;

I think it would be nice to add some documentation around mOriginalOptions and mOptions as to why they are needed, so we can have it at a glance in future.
Attachment #8941137 - Flags: review?(standard8) → review+
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/cba9a29be139
Make Places queries directly inherit options from their parent. r=standard8
Priority: P2 → P1
https://hg.mozilla.org/mozilla-central/rev/cba9a29be139
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Depends on: 1432746
Depends on: 1462738
You need to log in before you can comment on or make changes to this bug.