Synchronize /suite and /toolkit <contentAreaUtils.js>

RESOLVED FIXED

Status

SeaMonkey
General
--
minor
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

(Depends on: 2 bugs)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

10 years ago
 
(Assignee)

Comment 1

10 years ago
Created attachment 290401 [details] [diff] [review]
(Av1-TK) whitespace/++ cleanup

Whitespaces, comments and 1 code rewrite: no functional change.
Assignee: general → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #290401 - Flags: review?(mano)
(Assignee)

Comment 2

10 years ago
Created attachment 290402 [details] [diff] [review]
(Bv1-SM) whitespace cleanup and simple code sync

Simple TK->SM code sync...
Attachment #290402 - Flags: superreview?(neil)
Attachment #290402 - Flags: review?(neil)

Comment 3

10 years ago
Comment on attachment 290402 [details] [diff] [review]
(Bv1-SM) whitespace cleanup and simple code sync

>-function isContentFrame(aFocusedWindow)
>+function isContentFrame(aFocusedWindow)
I don't think it's worth moving entire functions around.

>+const kSaveAsType_Complete = 0; // Save document with attached objects.
>+// const kSaveAsType_URL      = 1; // Save document or URL by itself.
>+const kSaveAsType_Text     = 2; // Save document, converting to plain text. 
These should go where the old consts were. You've also got a trailing space.

>     contentType : (!aChosenData && useSaveDocument &&
>-                   saveAsType == SAVETYPE_TEXT_ONLY) ?
>-                  "text/plain" : null,
>+                   saveAsType == kSaveAsType_Text) ?
>+                   "text/plain" : null,
The second change is wrong - saveAsType lines up with ! but " with (.
Attachment #290402 - Flags: superreview?(neil)
Attachment #290402 - Flags: superreview-
Attachment #290402 - Flags: review?(neil)
(Assignee)

Comment 4

10 years ago
(In reply to comment #3)
> I don't think it's worth moving entire functions around.

It helps (me) when doing a diff between the files, but as you want.

> These should go where the old consts were. You've also got a trailing space.

All right. Missed that space.

> The second change is wrong - saveAsType lines up with ! but " with (.

Right, I didn't pay enough attention to this one.
(Assignee)

Comment 5

10 years ago
Created attachment 290596 [details] [diff] [review]
(Av1a-TK) whitespace/++ cleanup [Checkin: Comment 12]

Av1-TK, with comment 3 update,
and better documentation.
Attachment #290401 - Attachment is obsolete: true
Attachment #290596 - Flags: review?(mano)
Attachment #290401 - Flags: review?(mano)
(Assignee)

Comment 6

10 years ago
Created attachment 290597 [details] [diff] [review]
(Bv2-SM) whitespace cleanup and simple code sync [Checkin: Comment 7]

Bv1-SM, with comment 3 update.
Attachment #290402 - Attachment is obsolete: true
Attachment #290597 - Flags: superreview?(neil)
Attachment #290597 - Flags: review?(neil)

Updated

10 years ago
Attachment #290597 - Flags: superreview?(neil)
Attachment #290597 - Flags: superreview+
Attachment #290597 - Flags: review?(neil)
Attachment #290597 - Flags: review+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
Bv2-SM:

Checking in suite/common/contentAreaUtils.js;
/cvsroot/mozilla/suite/common/contentAreaUtils.js,v  <--  contentAreaUtils.js
new revision: 1.150; previous revision: 1.149
done
Keywords: checkin-needed
Attachment #290597 - Attachment description: (Bv2-SM) whitespace cleanup and simple code sync → (Bv2-SM) whitespace cleanup and simple code sync [checked in]
(Assignee)

Updated

10 years ago
Attachment #290597 - Attachment description: (Bv2-SM) whitespace cleanup and simple code sync [checked in] → (Bv2-SM) whitespace cleanup and simple code sync [Checkin: Comment 7]
(Assignee)

Updated

10 years ago
Depends on: 346994
Comment on attachment 290596 [details] [diff] [review]
(Av1a-TK) whitespace/++ cleanup [Checkin: Comment 12]

r=mano
Attachment #290596 - Flags: review?(mano) → review+
(Assignee)

Comment 9

10 years ago
Comment on attachment 290596 [details] [diff] [review]
(Av1a-TK) whitespace/++ cleanup [Checkin: Comment 12]

"approvalM10=?":
No code change, sync' only;
not mandatory, yet no need to wait either.
Attachment #290596 - Flags: approvalM10?
Comment on attachment 290596 [details] [diff] [review]
(Av1a-TK) whitespace/++ cleanup [Checkin: Comment 12]

No need to get this in for M10... just request approval1.9 for landing after beta.

Comment 11

10 years ago
Comment on attachment 290596 [details] [diff] [review]
(Av1a-TK) whitespace/++ cleanup [Checkin: Comment 12]

Can you land this *after* the tree re-opens from Beta2?
Attachment #290596 - Flags: approvalM10?
Attachment #290596 - Flags: approvalM10-
Attachment #290596 - Flags: approval1.9+
Keywords: checkin-needed
Checking in toolkit/content/contentAreaUtils.js;
/cvsroot/mozilla/toolkit/content/contentAreaUtils.js,v  <--  contentAreaUtils.js
new revision: 1.100; previous revision: 1.99
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Attachment #290596 - Attachment description: (Av1a-TK) whitespace/++ cleanup → (Av1a-TK) whitespace/++ cleanup [Checkin: Comment 12]
(Assignee)

Updated

10 years ago
Depends on: 408819
(Assignee)

Updated

10 years ago
Depends on: 409061
(Assignee)

Updated

10 years ago
Depends on: 409112
You need to log in before you can comment on or make changes to this bug.