123824 - idl documentation is poorly worded

Status: RESOLVED INACTIVE

(Core :: DOM: Core & HTML, defect, P5)

Description

[timeless]

i was browsing http://unstable.elemental.com/mozilla/build/latest/mozilla/dom/dox/interfacensIDOMWindow.html

And read:
attribute float nsIDOMWindow::textZoom   Set/Get the document scale 
factor. 

When setting this attribute, a NS_ERROR_NOT_IMPLEMENTED error may be 
returned by implementations not supporting zoom. Implementations not 
supporting zoom should return 1.0 all the time for the Get operation. 
1.0 is equals normal size, i.e. no zoom. 

-- This text needs to be at least as good as the implementation text, and it currently isn't.

Set/Get the document text scaling factor. 

An implementation that does not support zoom should return  NS_ERROR_NOT_IMPLEMENTED when the setter is called and the outvalue for the getter should be 1.0.

A textZoom of 1.0 means normal size, ie no scaling.

Comment 1

[Fabian Guisset]

timeless is right :-)

Comment 2

[Sören 'Chucker' Kuklau (gone)]

let's see

Comment 3

[Sören 'Chucker' Kuklau (gone)]

Attachment: my first take

Couldn't find much wrong with the original text, but reworded it anyway.

Comment 4

[Sören 'Chucker' Kuklau (gone)]

taking

Comment 5

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 110354 [details] [diff] [review]
my first take

> Couldn't find much wrong with the original text, but reworded it anyway.

That is silly.	Don't do that.	Or explain yourself better.  Doing things for
the hell of it is bad.	But more on that later....

Don't you want to get reviews from the people who actually own this file?  Like
dougt and darin.  One (both?) of them should at least see this patch (or a
revised version of it) before it gets reviewed.


>@@ -42,6 +42,7 @@
>  * methods" are not scriptable. 
>  *
>  * @status FROZEN
>+ *

Is there any particular reason for this change?

>  */
> [scriptable, uuid(c8c0a080-0868-11d3-915f-d9d889d48e3c)]
> interface nsIFile : nsISupports
>@@ -51,7 +52,9 @@
>      *
>      *  NORMAL_FILE_TYPE - A normal file.
>      *  DIRECTORY_TYPE   - A directory/folder.
>+     *
>      */
>+     

Or these changes?  Especially the new line after the comment, which goes
against style and screws up some JavaDoc converters which require there to be
no space between the comment and what it's commenting.	When making style
changes, do so because most of the file does it, not because you like it better
(or whatever your reasoning was).  This applies to pretty much every change you
made in the file.

>     const unsigned long NORMAL_FILE_TYPE = 0;
>     const unsigned long DIRECTORY_TYPE   = 1;
> 

>@@ -276,48 +367,69 @@
>      *       error (NS_ERROR_FILE_UNKNOWN_TYPE).
>      *
>      *   @param permissions
>-     *       The unix style octal permissions.  This may
>+     *       The unix style octal permissions. This may

Don't change this sort of thing.  I see several other places which use two
spaces between sentences, yet you do nothing to those places.  Follow the
prevailing style of the file (or just save yourself trouble and leave these
alone, it's not that big of a deal.  Really).  And read
http://java.sun.com/j2se/javadoc/writingdoccomments/ .	I suggest you write a
new patch, and then find more appropriate reviewers.

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Also, um, what happened to nsIDOMWindow?  I think I see now why you thought you
wanted jst and fabian to review this since the bug right up to your patch talked
about DOM stuff, but did you attach the wrong patch?

Comment 7

[Sören 'Chucker' Kuklau (gone)]

Comment on attachment 110354 [details] [diff] [review]
my first take

Duh, wrong patch. Sorry!

Comment 8

[Sören 'Chucker' Kuklau (gone)]

My time is very limited right now.

Comment 9

[Firefox Bug Husbandry Bot]

Bulk priority change, per :mdaly