nsIDocShell Browser/App attributes bikeshedding party

RESOLVED DUPLICATE of bug 802366

Status

()

Core
Document Navigation
RESOLVED DUPLICATE of bug 802366
6 years ago
5 years ago

People

(Reporter: mounir, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(blocking-basecamp:-)

Details

(Whiteboard: [leave open])

Attachments

(1 attachment)

5.50 KB, patch
Justin Lebar (not reading bugmail)
: review+
mounir
: checkin+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
Follow-up from bug 774957.

We should discuss the naming of the attributes/methods here... if we want to.
The only thing I really care about is that the two properties that map to the security model have sane names that we can refer to in all security model docs. That way code is easier to understand for people that are implementing and using the security model.

Hence I would like there to be a property .appId which returns the app we are currently "under", and a .inBrowser or .inBrowserElement or .browserContent which returns true iif we are inside a non-app <iframe mozbrowser>.

I'll leave all other names up to the docShell peers.
blocking-basecamp: --- → -
(Reporter)

Comment 2

6 years ago
Created attachment 644505 [details] [diff] [review]
First blood
Attachment #644505 - Flags: review?(justin.lebar+bug)
>diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
>--- a/docshell/base/nsDocShell.cpp
>+++ b/docshell/base/nsDocShell.cpp
>@@ -12021,17 +12021,17 @@ nsDocShell::GetCanExecuteScripts(bool *a
> #endif // DEBUG
>       } while (treeItem && docshell);
>   }
> 
>   return NS_OK;
> }
> 
> NS_IMETHODIMP
>-nsDocShell::SetIsBrowser()
>+nsDocShell::SetIsBrowserElement()
> {
>     if (mIsBrowserFrame) {
>         NS_ERROR("You should not call SetIsBrowser() more than once.");

Fix this.
Attachment #644505 - Flags: review?(justin.lebar+bug) → review+
(Reporter)

Comment 4

6 years ago
(In reply to Justin Lebar [:jlebar] from comment #3)
> >diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
> >--- a/docshell/base/nsDocShell.cpp
> >+++ b/docshell/base/nsDocShell.cpp
> >@@ -12021,17 +12021,17 @@ nsDocShell::GetCanExecuteScripts(bool *a
> > #endif // DEBUG
> >       } while (treeItem && docshell);
> >   }
> > 
> >   return NS_OK;
> > }
> > 
> > NS_IMETHODIMP
> >-nsDocShell::SetIsBrowser()
> >+nsDocShell::SetIsBrowserElement()
> > {
> >     if (mIsBrowserFrame) {
> >         NS_ERROR("You should not call SetIsBrowser() more than once.");
> 
> Fix this.

Bug 776195
(Reporter)

Updated

6 years ago
Whiteboard: [leave open]
(Reporter)

Updated

6 years ago
Attachment #644505 - Flags: checkin+
> > >     if (mIsBrowserFrame) {
> > >         NS_ERROR("You should not call SetIsBrowser() more than once.");
> > 
> > Fix this.
> >
> Bug 776195

Sorry, I just meant that it should say /SetIsBrowserElement/.
https://hg.mozilla.org/mozilla-central/rev/3dfc809486f5
(Reporter)

Comment 7

6 years ago
(In reply to Justin Lebar [:jlebar] from comment #5)
> > > >     if (mIsBrowserFrame) {
> > > >         NS_ERROR("You should not call SetIsBrowser() more than once.");
> > > 
> > > Fix this.
> > >
> > Bug 776195
> 
> Sorry, I just meant that it should say /SetIsBrowserElement/.

Just pushed something to fix that.
https://hg.mozilla.org/mozilla-central/rev/c07793b7dc10
This is basically subsumed by bug 802366.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 802366
You need to log in before you can comment on or make changes to this bug.