Closed Bug 542912 Opened 10 years ago Closed 10 years ago

Refactor the newline handling behavior in nsTextEditRules

Categories

(Core :: DOM: Editor, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Patch (v1) (obsolete) — Splinter Review
This patch refactors out the newline handling behavior in nsTextEditRules, and also makes FillBufWithPWChars accessible to outside callers.
Attachment #424148 - Flags: superreview?(roc)
Attachment #424148 - Flags: review?(bzbarsky)
Blocks: 542919
Comment on attachment 424148 [details] [diff] [review]
Patch (v1)

+  /**
+   * Handles the newline characters either according to aNewLineHandling
+   * or to the default system prefs if aNewLineHandling is negative.
+   */

Can you document what aNewLineHandling is allowed to be, and what those values do to aString?
Attachment #424148 - Flags: superreview?(roc) → superreview+
Comment on attachment 424148 [details] [diff] [review]
Patch (v1)

>+++ b/editor/libeditor/text/nsPlaintextEditor.cpp
>+nsPlaintextEditor::GetDefaultEditorPrefs(PRInt32 &aNewlineHandling,

If the prefs are not set, this function doesn't change its inout params.  It should probably set them to the default values nsPlaintextEditor uses instead.

>+++ b/editor/libeditor/text/nsTextEditRules.h
>+  /** Echoes the insertion text into the password buffer, and converts
>+      insertion text to '*'s */                                        

This doesn't make any sense.  What does this function actually do?

r=me with those two things fixed.
Attachment #424148 - Flags: review?(bzbarsky) → review+
Attached patch Patch (v1)Splinter Review
(In reply to comment #1)
> (From update of attachment 424148 [details] [diff] [review])
> +  /**
> +   * Handles the newline characters either according to aNewLineHandling
> +   * or to the default system prefs if aNewLineHandling is negative.
> +   */
> 
> Can you document what aNewLineHandling is allowed to be, and what those values
> do to aString?

Done.

(In reply to comment #2)
> (From update of attachment 424148 [details] [diff] [review])
> >+++ b/editor/libeditor/text/nsPlaintextEditor.cpp
> >+nsPlaintextEditor::GetDefaultEditorPrefs(PRInt32 &aNewlineHandling,
> 
> If the prefs are not set, this function doesn't change its inout params.  It
> should probably set them to the default values nsPlaintextEditor uses instead.

Done.

> >+++ b/editor/libeditor/text/nsTextEditRules.h
> >+  /** Echoes the insertion text into the password buffer, and converts
> >+      insertion text to '*'s */                                        
> 
> This doesn't make any sense.  What does this function actually do?

I documented this function in a saner way.

Updated patch attached.
Attachment #424148 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/d5e4ed59c450
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
You need to log in before you can comment on or make changes to this bug.