clarify nsIWebBrowserSetup documentation

VERIFIED FIXED in mozilla1.1alpha

Status

()

Core
Embedding: APIs
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: David Epstein, Assigned: Adam Lock)

Tracking

Trunk
mozilla1.1alpha
x86
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

3.38 KB, patch
David Epstein
: review+
rpotts (gone)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

16 years ago
Mozilla 1.0.0 Gecko/20020507.
1. Launch testEmbed from /bin directory (compile nmake -f makefile.win from
/embedding/qa/testembed folder).
2. Select Tools > TestYourMethod. Currently, the test code is there. Will be
moved later on and will indicate new location.
3. Blast through the modal OKs.
4. Notice that SetProperty() returns rv passed for all cases (2nd parameter is
PR_TRUE or PR_FALSE).
5. Now enter some other values for the 2nd aValue parameter. Try "1.5" and
"2.0". Recompile and relaunch.

source:
	nsCOMPtr <nsIWebBrowserSetup> qaWBSetup(do_QueryInterface(qaWebBrowser, &rv));
	RvTestResult(rv, "nsIWebBrowserSetup object test", 2);

	rv = qaWBSetup->SetProperty(nsIWebBrowserSetup::SETUP_ALLOW_PLUGINS, 2);
	RvTestResult(rv, "nsIWBSetup:SetProperty(SETUP_ALLOW_PLUGINS,2)", 2);

Result: ANY value less than 2.0 works. (e.g. 1.99).
Expected: Either consistent boolean usage screening out all other values, or
some explained functional associated values for 2nd parameter.
(Reporter)

Comment 1

16 years ago
change qa contact to depstein
QA Contact: mdunn → depstein
(Assignee)

Comment 2

16 years ago
The method nsIWebBrowserSetup::setProperty is defined like this:

void setProperty(in unsigned long aId, in unsigned long aValue);

The property id is a long, and the value is a longs. Longs are integers so
supplying reals such as 1.5 will automatically be cast into a long by the
compiler. The permissible values are property specific but most of our
properties are booleans and should reject values other than PR_TRUE(1) and
PR_FALSE(0).

A glance at the impl of this interface suggests that we are doing the property
checking to ensure bad values are caught.

http://lxr.mozilla.org/seamonkey/source/embedding/browser/webBrowser/nsWebBrowser.cpp#690
(Reporter)

Comment 3

16 years ago
OK, so it appears the properties are recasting to 0 or 1 depending on the value
between 0 and 2. All the property IDs, with the exception of
SETUP_FOCUS_DOC_BEFORE_CONTENT (which the idl file says is obsolete) return rv
false for values >= 2. So if this is intended behavior, I'd take it to mean that
all the properties accept only boolean on/off switching. Otherwise, an
individual property might accept values >= 2 as well, which doesn't appear to be
the case wrt the enabling/disabling descriptions in the idl.
(Assignee)

Comment 4

16 years ago
Documentation for nsIWebBrowserSetup should make clear which of these properties
are actually booleans.
Summary: some inconsistencies with nsIWebBrowserSetup->SetProperty aValues → clarify nsIWebBrowserSetup documentation
(Assignee)

Updated

16 years ago
Target Milestone: --- → mozilla1.1alpha
(Assignee)

Comment 5

16 years ago
Created attachment 85786 [details] [diff] [review]
Updated comments

Looking for a quick r/sr on these documentation only changes

Comment 6

16 years ago
Comment on attachment 85786 [details] [diff] [review]
Updated comments

sr=rpotts@netscape.com
Attachment #85786 - Flags: superreview+
(Assignee)

Comment 7

16 years ago
Ellen or David, can you review the changes please?
(Reporter)

Updated

16 years ago
Attachment #85786 - Flags: review+
(Reporter)

Comment 8

16 years ago
Comment on attachment 85786 [details] [diff] [review]
Updated comments

(From update of attachment 85786 [details] [diff] [review])
r=depstein@netscape.com
(Assignee)

Comment 9

16 years ago
Fix checked in
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 10

16 years ago
Changes now reflected in API ref
(Reporter)

Comment 11

16 years ago
verified. Mozilla 1.1a Gecko/20020607 build.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.