147730 - Upload dialog shows painting issue when moving scrollbars

Status: VERIFIED FIXED

(Camino Graveyard :: General, defect)

Description

[Chris Petersen]

Build: 0.2.8
Platform: OS X 10.1.4
Expected Results: No paint issues in dialog after dragging scrollbar
What I got: Position of scrollbar is shifted

Steps to reproduce:

1) Open url and click on browse button.
2) Drag scrollbar up or down in dialog.
3) Scrollbar shifts. See attacment.

Comment 1

[Chris Petersen]

Attachment: screen shot

Comment 2

[Chris Petersen]

There are three form bugs that give the user a less than spectacular experience
in Chimera when filling out a bug report:  

This current bug of attempting to attach a file:
http://bugzilla.mozilla.org/show_bug.cgi?id=147730

Input (type="file") field not rendering content:
http://bugzilla.mozilla.org/show_bug.cgi?id=147739

Scrollable areas don't scroll:
http://bugzilla.mozilla.org/show_bug.cgi?id=144874

Comment 3

[saari (gone)]

->pinkerton

Comment 4

[Mike Pinkerton (not reading bugmail)]

i wonder if this is a case of trying to use a carbon file picker from w/in a
cocoa app.

over to dagley to help load-balance.

Comment 5

[Greg K.]

Confirmed using Chimera/20020619.

Comment 6

[Brian Ryner (not reading)]

*** Bug 155042 has been marked as a duplicate of this bug. ***

Comment 7

[David Haas (gone, not reading mail)]

Attachment: quick & dirty

Using 6-29 NB, I see this problem.  

For grins, I made a (very) quick & dirty re-write of
mozilla/widget/src/cocoa/nsFilePicker.cpp
to use NSOpenPanel rather than carbon NavServices. I
used that build to attach the following bug report.  
I hope the patch (which contains the cruft) actually 
makes it here . . .

Anyway, the funny dialog painting issues went away.  
So I think comment #4 is right on the money.

So, a dumb question: is there a problem with rewriting cocoa/nsFilePicker
to actually use the Cocoa file picker?	I guess I'm wondering if Mozilla
would have a problem with it, rather than Chimera, assuming things ever
got merged back into the main mozilla branch?

If not, I (or somebody else) can fix up some of the cruft that's missing
and get this in.

Comment 8

[Steve Dagley]

David, the only reasons cocoa/nsFilePicker hadn't been been converetd to Cocoa
yet is time and more pressing bugs to fix.  Your patch goes a long way to
addressing it though other folks are still too busy at the moment.  If you could
implement the PutLocalFile method (and I'll mention you can't use InitWithFSRef
for non-existant files since FSRef's can't represent those) and re-submit the
patch I think pinkerton would be willing to review it and land it as better
behavior than what we have now.

Comment 9

[Mike Pinkerton (not reading bugmail)]

ccing conrad and smfr so when the new patch comes in, we can get eyeballs on it.
keep up the great work, dave!

Comment 10

[Conrad Carlen (not reading bugmail)]

Comment on attachment 89880 [details] [diff] [review]
quick & dirty 

Awesome. So much more concise in Cocoa :-) And, not only will this fix the
painting problems, but won't it allow keyboard navigation of the dialog (broken
in Carbon)?

>+//-------------------------------------------------------------------------
>+NS_IMETHODIMP nsFilePicker::GetFileURL(nsIFileURL **aFileURL)
>+{
>+  NS_ENSURE_TRUE(mFile, NS_ERROR_FAILURE);
>+

I think the preferred way to do this is here:
http://lxr.mozilla.org/mozilla/source/netwerk/base/public/nsNetUtil.h#755

>+  nsCOMPtr<nsIURI> uri;
>+  NS_NewFileURI(getter_AddRefs(uri), mFile);
>+  nsCOMPtr<nsIFileURL> fileURL(do_QueryInterface(uri));
>+  NS_ENSURE_TRUE(fileURL, NS_ERROR_FAILURE);
>+  
>+  NS_ADDREF(*aFileURL = fileURL);
>+  return NS_OK;
>+}

Comment 11

[David Haas (gone, not reading mail)]

Hey - could somebody point out a couple of links to test out
the get folder & put file functions?  something besides bugzilla
to test out get file would be nice, too.

Comment 12

[Steve Dagley]

I don't believe we expose any UI for Get Folder in Chimera but Put File can be
exercised by doing a control-click/right-click in the body of a page and
selecting "Save Page As...".  Note that the "Save As..." item under the File
menu does not use the File Picker to display its dialog.

Comment 13

[Conrad Carlen (not reading bugmail)]

Sorry, ignore my comment about GetFileURL. I misread completely :-/

Comment 14

[David Haas (gone, not reading mail)]

Hmm.  So I made a cocoa-ized PutLocalFile function.  I planned on trying
it using the control-click/"Save Page As" context menu.  
It didn't get called - so I don't know if it works. 

Looks like (based on some older NB's) that "Save Page As" context 
menu already calls up a cocoa save panel - at least you don't see 
any painting issues with it. 

Is there anyplace/way somebody else could mention to try out PutLocalFile?
Or might this just be a dead routine?  

Comment 15

[David Haas (gone, not reading mail)]

Attachment: cocoa file picker

This patch implements GetFile, GetFolder, and PutFile.

As near as I can tell, Chimera only uses GetFile.  So GetFolder
& PutFile haven't been tried out at all.  

If you apply this, take nsFilePicker.cpp out of mozilla/widget/src/cocoa

Comment 16

[Greg K.]

David, does this latest patch obsolete your Q&D patch?

Comment 17

[David Haas (gone, not reading mail)]

Yes, it obsoletes q&d patch.  

Just out of curiosity, is there a reason I can't obsolete my own patches?

Comment 18

[Greg K.]

You need to ask Asa Dotzler for bug editing privs. You can reach him by e-mail
at [[e-mail]].

Comment 19

[Greg K.]

Comment on attachment 89880 [details] [diff] [review]
quick & dirty 

Per comment #17.

Comment 20

[Simon Fraser [no longer active]]

Does this patch build? I get:

In file included from nsFilePicker.cpp:41:
nsFilePicker.h:38: parse error before `@'

Comment 21

[Simon Fraser [no longer active]]

Never mind, removing nsFilePicker.cpp fixed it.

Comment 22

[Simon Fraser [no longer active]]

Attachment: Testing attachment, please ignore

Comment 23

[Simon Fraser [no longer active]]

taking

Comment 24

[Simon Fraser [no longer active]]

Patch checked in, after some slight cleanup by me.

Note to QA: this code is *not* used when doing Open or Save in the browser
window, but only when doing things that open dialogs from browser content (like
file Inputs etc).

Comment 25

[Chris Petersen]

Attachment: Cool, this is fixed

Comment 26

[Chris Petersen]

No scrollbar problems occured when attaching sample file to this report. Marking
verified with 2002-07-18-05 NB.