Closed Bug 267219 Opened 16 years ago Closed 15 years ago

[BEOS] Selecting a directory fails and is uninituitive

Categories

(Core Graveyard :: GFX: BeOS, defect)

x86
BeOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Niels.Reedijk, Assigned: Niels.Reedijk)

References

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.7.3) Gecko/20041028 Firefox/1.0RC1
Build Identifier: Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.7.3) Gecko/20041028 Firefox/1.0RC1

When using a directory selector, the directory isn't stored when it is selected.
Furthermore, opening a directory is uninituitive. 

This bug is the beos cousin of bug 266252 .

Reproducible: Always
Steps to Reproduce:
Blocks: 266252
This is a first patch of mine (and automagically a first try at this bug).
Check out it's brother bug report for the complete problem description. 

Only thing I'm not completely sure about is where to put the MSG_DIRECTORY
message definition. Any hints? If that question is answered, I'll put it up for
review.

Niels
QA Contact: jrgmorrison → Niels.Reedijk
Assignee: jag → beos
Component: XP Toolkit/Widgets → GFX: BeOS
QA Contact: Niels.Reedijk → timeless
niels: please handle the cases when new fails. (believe me, this does happen)
confirming
Status: UNCONFIRMED → NEW
Ever confirmed: true
Where should I put:
 const uint32 MSG_DIRECTORY = 'mDIR';
Can I just put it in the implementation? It's used only there. Or should I
#define it? 
BTW, at the moment this patch contains a buffer overflow when the name and the
label are larger than 50 characters. I'll have that fixed as well.
Attached patch Updated patch (obsolete) — Splinter Review
Remove debug output and check if new fails.
Attachment #164234 - Attachment is obsolete: true
Attachment #165247 - Flags: superreview?(sergei_d)
Attachment #165247 - Flags: review?(thesuckiestemail)
Comment on attachment 165247 [details] [diff] [review]
Updated patch

Obsolete patch
Attachment #165247 - Attachment is obsolete: true
Attachment #165247 - Flags: superreview?(sergei_d)
Attachment #165247 - Flags: review?(thesuckiestemail)
I moved the MSG_DIRECTORY definiton to the implementation. I wonder whether
this is te right place to put it, and if the way of defining it (as a const
uint32) is good as well. I haven't found any other messages in the source yet,
so I'm wondering what I should do with this one.
Attachment #165248 - Flags: superreview?(sergei_d)
Attachment #165248 - Flags: review?(thesuckiestemail)
Niels, for future:
1)Ask for superreview when review is already granted.
2)In this case, as we change files in BeOS-only folder, we don't need superreview.

Also, about patch itself.
Isn't more reasonable to use B_PATH_NAME_LENGTH instead B_FILE_NAME_LENGTH ?
(I cannot test/inestigate real code at the moment, so asking)
(In reply to comment #9)
> Niels, for future:
> 1)Ask for superreview when review is already granted.
> 2)In this case, as we change files in BeOS-only folder, we don't need superreview.

I'll keep that in mind.

> Also, about patch itself.
> Isn't more reasonable to use B_PATH_NAME_LENGTH instead B_FILE_NAME_LENGTH ?
> (I cannot test/inestigate real code at the moment, so asking)

No, it's not. The string is 10 bytes long (including the \0), plus the maximum
length of a filename. There isn't any path involved, so this one is more logical.
Only obstacle i see here, is i18n. Inability to autochange button label according 
 to user language. But that's another problem.
i18n doesn't work on BeOS anyway, unless you perform serious voodoo, so breaking
it even more is not an issue. No XPI the Irish Localisation Project has ever
generated for Firefox has worked.
(In reply to comment #12)
> i18n doesn't work on BeOS anyway, unless you perform serious voodoo, so breaking
> it even more is not an issue. No XPI the Irish Localisation Project has ever
> generated for Firefox has worked.

Furthermore, the dialog is in English (for another three years anyway, minimum).
So it would be quite strange to have an english dialog with a "<map> selecteren"
button.
As i said, this problem isn't important, just had wish to notice that witj
localized Tracker version FilePanel will/may look bit strange.
And anyway, current Mozilla/Firefox nsFilePicker BeOS implementation is real crap. 

But i really prefer don't see here such incompetent  "arguments" as MYOB's
comment. It can compromise even really perfect patch and even occasionally
trigger on veto from mozilla VIPs.
I'll try looking at the i18n functions that are available. If it is doable, then
I'll update the patch. The problem is, that we'll have to work in unicode, and I
don't know how that can be transfered to a bbutton.
Don't tinker with it now.
See, this nsFilePicker is messy object - half-native.
And we miss in BeOS atm established localization concept.
At least until Haiku is out and has unified with Zeta localization API.

And mozilla generic localization works on XUL level, which is also very hard to
connect to our case. Workaround may be bitmap-buton but i'm not aware about any
established icon for "Select" action.

So if people like that option and tqh will put review at that, my sr comes
automatically.
I agree with fyysik's comments, but that maybe should be handled in a future
bug. this patch looks like a nice improvement.

I've looked at the code and it looks ok, but I want to setup a tree and test it
too before I review it. Hopefully I will be able to do it today or tomorrow (19/11).

Comment on attachment 165248 [details] [diff] [review]
Move MSG_DIRECTORY to implementation

r=thesuckiestemail@yahoo.se

Looks ok, works ok. Although I don't like the prefs for selecting
download-folder under BeOS,
 it seems to be quite flawed
Attachment #165248 - Flags: review?(thesuckiestemail) → review+
Comment on attachment 165248 [details] [diff] [review]
Move MSG_DIRECTORY to implementation

sr
Attachment #165248 - Flags: superreview?(sergei_d) → superreview+
Comment on attachment 165248 [details] [diff] [review]
Move MSG_DIRECTORY to implementation

Requesting aproval as trunk is frozen. This touches BeOS only-code, and will
not affect any platforms in any way.
Attachment #165248 - Flags: approval1.8a5?
Comment on attachment 165248 [details] [diff] [review]
Move MSG_DIRECTORY to implementation

a=asa for trunk checkin.
Attachment #165248 - Flags: approval1.8a5? → approval1.8a5+
Attachment #165248 - Flags: approval-aviary?
Comment on attachment 165248 [details] [diff] [review]
Move MSG_DIRECTORY to implementation

1.0 has shipped and we're looking to the trunk for future Firefox releases.
Attachment #165248 - Flags: approval-aviary? → approval-aviary-
Assignee: beos → Niels.Reedijk
Status: NEW → ASSIGNED
Timeless: could you check this in?
fix checked in
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.