Updates to nsLocalFileOS2

RESOLVED FIXED

Status

()

Core
XPCOM
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: Rich Walsh, Assigned: dougt)

Tracking

Trunk
x86
OS/2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

13 years ago
The patch brings the OS/2 version of nsLocalFile up to a code level comparable
to its Win32 counterpart.



In general, new or revised Win32 code was copied as-is, while existing OS/2 code
was largely left intact.  Reformatting (to provide a consistent coding style)
and rearrangement (to group all static functions at the top of the file) account
for the bulk of the diff.  To make review a little easier, I've attached an
extra diff that ignores whitespace changes.



New or significantly revised code:

 *  _mbsstr() - required by AppendNativeInternal() [original]

 *  nsDriveEnumerator - patterned after the Win32 version [original]

 *  AppendNativeInternal() - supports AppendNative() and
      AppendRelativeNativePath [ported]

 *  Normalize() - previously a no-op [ported]

 *  SetPermissions() - neither the OS/2 nor the Win32 versions seemed
      appropriate;  I rewrote this to ensure that only the readonly flag
      will ever be changed [original]

 *  IsExecutable() - again, neither the OS/2 nor Win32 versions seemed
      quite right [original]



There are two other minor changes worth noting:

 *  Reveal() - now uses OPEN_DEFAULT to honor the user's choice of views
      (this resolves a personal grievance)

 *  IsSpecial() - now returns false;  in the future, it will return true
      if the "file" is a WPS object.  (N.B.  Linux uses this method to
      identify non-file objects such as devices and sockets.  No other
      platform-specific or XP code ever calls this method.)
(Reporter)

Comment 1

13 years ago
Created attachment 183529 [details] [diff] [review]
diff for checkin (includes whitespace changes)
(Reporter)

Comment 2

13 years ago
Created attachment 183530 [details] [diff] [review]
review diff (excludes whitespace changes)

not for check-in

Comment 3

13 years ago
I'll probably have to take your word on this :)

If the browser comes up and generally works, you got it right...

Comment 4

13 years ago
Comment on attachment 183529 [details] [diff] [review]
diff for checkin (includes whitespace changes)

Even the review version is quite hard to follow, still lots of formatting
changes intermingled with real changes to the code, but I hope I have found and
checked the important changes. Well, I don't really have any comments, the
suite still runs nicely with this patch. Or should I verify that anything now
works better than before? (Well, the SetPermissions change might help with the
multiple bookmarks problem of Firefox...)
Attachment #183529 - Flags: superreview?(mozilla)
Attachment #183529 - Flags: review+

Comment 5

13 years ago
Comment on attachment 183529 [details] [diff] [review]
diff for checkin (includes whitespace changes)

These changes were shipped as part of the recent Dee Park alpha and we recived
no negative feedback.
Attachment #183529 - Flags: superreview?(mozilla)
Attachment #183529 - Flags: superreview+
Attachment #183529 - Flags: approval1.8b3+

Comment 6

13 years ago
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.