Rename QueryContext to UrlbarQueryContext

RESOLVED FIXED in Firefox 66

Status

()

enhancement
P3
normal
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: adw, Assigned: adw)

Tracking

(Blocks 1 bug)

unspecified
Firefox 66
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

5 months ago

Most class names start with "Urlbar", but QueryContext doesn't, so we should rename it to be consistent. Marco suggests UrlbarContext -- leaving out "Query" -- since we've added many properties to it and some aren't necessarily strictly related to a query, and the term "query" is used elsewhere and may be confusing.

I'm marking this blocking the main quantumbar bug, but maybe it's better for it to block the three model, view, and controller bugs? Feel free to change.

Assignee

Comment 1

5 months ago

There are still lots of local uses of "query context" and "queryContext", and I didn't change them. In some places those terms still makes sense because the code is clearly dealing with a query/search. In other places they make less sense, but this patch ended up being a lot bigger than I expected and I didn't really want to change more blame for this.

I think the UrlbarQueryContext name was nodded to, because UrlbarContext is yet again too generic. Just sayin'.

(In reply to Mike de Boer [:mikedeboer] from comment #2)

I think the UrlbarQueryContext name was nodded to, because UrlbarContext is yet again too generic. Just sayin'.

Yeah, I too think UrlbarContext is too ambiguous.

Assignee

Comment 4

5 months ago

I thought we ended up on UrlbarContext for the reasons in comment 0?

I'm in two minds about this. In some ways "Context" isn't the context for the whole urlbar (if it was, I think I'd expect it to be a singleton), it is the context for a particular query / search results. The fact it does contain other items I'm not sure matters too much - they're context for the search, and a different view could possibly vary these depending on what it fancies (silly or not(?) idea - time of day).

Assignee

Comment 6

5 months ago

OK, it sounds like most of us think Query should be in the name. I think I do too.

Ayes: Mike, Dão, Me, 1/2 of Mark
Noes: Marco (I think -- maybe I misunderstood?), 1/2 of Mark

I'll post a new revision using UrlbarQueryContext.

Assignee

Updated

5 months ago
Summary: Rename QueryContext to UrlbarContext → Rename QueryContext to UrlbarQueryContext
Attachment #9037380 - Attachment is obsolete: true

I'm fine either way, go for it.

Comment 9

5 months ago
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/efa95aa6623a
Rename QueryContext to UrlbarQueryContext. r=Standard8

Comment 10

5 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
You need to log in before you can comment on or make changes to this bug.