Closed
Bug 1294638
Opened 8 years ago
Closed 8 years ago
Remove appId and isInIsolatedMozBrowserElement from nsIDocShell because they're in nsILoadContext
Categories
(Core :: DOM: Navigation, defect)
Core
DOM: Navigation
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(1 file)
(deleted),
patch
|
bzbarsky
:
review-
|
Details | Diff | Splinter Review |
Bug 775860 duplicated two attributes from nsIDocShell into nsILoadContext.
Given that nsDocShell is the only class that implements nsIDocShell, *and* it
also implements nsILoadContext, we can remove the attributes from nsIDocShell.
Assignee | ||
Comment 1•8 years ago
|
||
I discovered this while modifying the code generation of infallible attributes;
having two identical attributes (one fallible, one infallible) used by
nsDocShell was causing some difficulties. Avoiding the duplication seems like a
good thing to do.
Attachment #8780407 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 2•8 years ago
|
||
Try looks reasonable: https://treeherder.mozilla.org/#/jobs?repo=try&revision=65a6c8123e48
I'm not sure if the combination of casting and QI'ing in the patch is optimal. Suggestions welcome.
Comment 3•8 years ago
|
||
so infallible doesn't work in nsILoadContext because it isn't builtinclass or what?
Assignee | ||
Comment 4•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #3)
> so infallible doesn't work in nsILoadContext because it isn't builtinclass
That's correct.
Comment 5•8 years ago
|
||
Comment on attachment 8780407 [details] [diff] [review]
Remove appId and isInIsolatedMozBrowserElement from nsIDocShell because they're in nsILoadContext
Conceptually, the attrs on loadcontext are information about a load, while the ones on docshell are information about a spec navigation context...
And in general, I think having docshell directly implement nsILoadContext was a mistake; we should have used a shim object that forwarded some stuff to the docshell. I would really much rather we tried to fix that mistake instead of enshrining it even more.
Attachment #8780407 -
Flags: review?(bzbarsky) → review-
Assignee | ||
Comment 6•8 years ago
|
||
Fair enough.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•