Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 225045 - [ActiveX] OLECMDID_PAGESETUP gives an exception
: [ActiveX] OLECMDID_PAGESETUP gives an exception
: fixed1.7
Product: Core Graveyard
Classification: Graveyard
Component: Embedding: ActiveX Wrapper (show other bugs)
: Trunk
: x86 Windows 2000
: -- normal (vote)
: ---
Assigned To: Adam Lock
: Michael Dunn
Depends on:
  Show dependency treegraph
Reported: 2003-11-07 19:36 PST by irongut
Modified: 2012-04-04 01:09 PDT (History)
1 user (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

Patch (19.76 KB, patch)
2004-03-18 12:43 PST, Adam Lock
no flags Details | Diff | Splinter Review
Patch for user prompting thing (978 bytes, patch)
2004-03-18 13:04 PST, Adam Lock
no flags Details | Diff | Splinter Review
Full patch (37.50 KB, patch)
2004-03-23 12:58 PST, Adam Lock
atremon: review+
Details | Diff | Splinter Review
Full patch mk2 (37.73 KB, patch)
2004-03-28 03:42 PST, Adam Lock
bzbarsky: superreview+
asa: approval1.7+
Details | Diff | Splinter Review

Description irongut 2003-11-07 19:36:24 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007
Build Identifier: 

OLECMDID_PAGESETUP is defined in MozillaBrowser.h but gives an exception.
Other commands that are defined work fine, e.g. OLECMDID_PRINT.
Code used works fine with IE based WebBrowser control.
Reproduced with control from v1.4, v1.4.1 and v1.5.

Reproducible: Always

Steps to Reproduce:
1. Embed Mozilla Control in an app
2. Call OLECMDID_PAGESETUP of IOleCommandTarget interface

Actual Results:  
In Delphi, got the following exception:
EOleException: Trying to revoke a drop target that has not been registered.

Expected Results:  
Display a Page Setup dialog.
Comment 1 Adam Lock 2004-03-15 14:14:02 PST
That's a strange error message.

The implementation of OLECMDID_PAGESETUP return S_OK but it will pop up a "No
page setup yet!" message before that. The function does nothing else.

Do you have a code sample of how you are calling it - perhaps it doesn't like
one of the args?
Comment 2 irongut 2004-03-17 12:52:42 PST
Yes it is a strange error message. Delphi seems to spawn that exception any time
it has problems with OLE. You even get it if you forget to initialise Delphi's
OLE handling code. I don't think the wording means much.

Here is my code, this works fine in the MS control:

procedure TfrmMain.mPageSetupClick(Sender: TObject);
{show Page Setup dialog}
    {ensure not busy or printing before showing dialog}
    if not(mzGecko.Busy) and (mzGecko.QueryStatusWB(OLECMDID_PAGESETUP) > 0)
      {show page setup dialog}
    {handle exceptions}
    on E : Exception do
      MessageDlg('ERROR: Unable to show Page Setup dialog. ' + #13 + E.ClassName
        + ': ' + E.Message + '.', mtError, [mbOk], 0);
    end; {try..except..}
end; {procedure TfrmMain.mPageSetupClick}

Most of the above is exception handling, the calling line is:


BTW, still getting this exception with v1.6.
Comment 3 Adam Lock 2004-03-17 14:38:32 PST
I don't know if specifying OLECMDEXECOPT_PROMPTUSER makes a difference - my code
doesn't test the args since it assumes page setup means prompting the user.

I might adopt this bug for a proper implementation of dialog. I'll also check to
see if there is anything about ExecWB which might cause this problem.
Comment 4 irongut 2004-03-17 17:38:42 PST
(In reply to comment #3)
> I don't know if specifying OLECMDEXECOPT_PROMPTUSER makes a difference - my
> code doesn't test the args since it assumes page setup means prompting the
> user.


Changed the flag to OLECMDEXECOPT_DODEFAULT and now I get the 'No page setup
yet!' message you expected.

I get the same exception for a number of other ExecWB commands. I just tried
OLECMDID_SAVEAS and that now works perfectly. Too busy to do more now, I'll try
the others next week and report in the relevant bugs.

Looks like you need to do something with the args for compatibility with the IE
control. :)
Comment 5 Adam Lock 2004-03-18 12:39:08 PST
I'm going to hijack this bug seeing as I've written page setup. I'll attach that
patch although it does not contain any fixes for this prompt user thingy.
Comment 6 Adam Lock 2004-03-18 12:43:44 PST
Created attachment 144259 [details] [diff] [review]

This patch implements the Page Setup dialog box. Functionality and layout is
identical to the page setup dialog in Firefox.
Comment 7 Adam Lock 2004-03-18 13:04:19 PST
Created attachment 144263 [details] [diff] [review]
Patch for user prompting thing

This is the patch for user prompting thing.

Basically I was testing for the OLECMDEXECOPT_SHOWHELP (value 3) thinking it
was a flag when it isn't.

This meant if you supplied OLECMDEXECOPT_PROMPTUSER (value 1) or anything else
to ExecWB, the test would fail and the code would fall out of the bottom with
an OLECMDERR_E_NOTSUPPORTED error. The patch changes the code to do a
Comment 8 Alexandre Trémon 2004-03-23 00:26:50 PST
Comment on attachment 144259 [details] [diff] [review]


I miss 
that are probably defined in the resource.h?

Isn't there anything changed in the .rc file?
Comment 9 Adam Lock 2004-03-23 12:58:21 PST
Created attachment 144597 [details] [diff] [review]
Full patch

This is the full patch containing the previous two plus the resource changes as
Comment 10 Adam Lock 2004-03-23 13:00:04 PST
Comment on attachment 144597 [details] [diff] [review]
Full patch

Full patch including resources
Comment 11 Alexandre Trémon 2004-03-24 03:00:27 PST
Comment on attachment 144597 [details] [diff] [review]
Full patch


* I had a problem applying the patch when in MozillaControl.rc

This is the ouput:
patching file MozillaControl.rc
Hunk #15 succeeded at 304 with fuzz 2 (offset -8 lines).
missing header for unified diff at line 365 of patch
can't find file to patch at input line 365
Perhaps you should have used the -p or --strip option?
The text leading up to this was:

I tried without success to call patch.exe with -p 0 option, so I applied the
remaining differences manually.

* In PageSetupDlg.h, line 146 & 312, my compiler didn't like _tstof. Are you
using VC7? I use VC6. I tried to replace my stlib.h and tchar.h with ones I got
from a MS SDK, but the compiler didn't like that either.
Do I have to change for vc7 (what is used by TinderBox?) ?
I changed _tstof() by atof(T2A()), and it works nicely.

* For PageSetupDlg.h: I know that in ATL a lot goes into .h files for
efficiency, but then, when you have to change one line, you have to compile a
lot more than if the code were in a .cpp

* I get an assertion because 'cData' in the call ::SendMessage(hwndTarget,
WM_COMMAND, LOWORD(pCI->nWindowsCmdID), (LPARAM) &cData); is at one point
interpreted as a HWND.
Comment 12 Alexandre Trémon 2004-03-24 03:10:34 PST
Comment on attachment 144597 [details] [diff] [review]
Full patch

* To detect memory leaks, I tried to use the leaky tool, but I have a lot of
trouble compiling it. Do you use a tool for mem leaks?

* You can add the PageSetupDlg.h file in the MozillaControl.dsp
Comment 13 irongut 2004-03-24 12:49:55 PST
(In reply to comment #4)
> I get the same exception for a number of other ExecWB commands. I just tried
> OLECMDID_SAVEAS and that now works perfectly. Too busy to do more now, I'll 
> try the others next week and report in the relevant bugs.

Double checked but other commands I had problems with are missing from
MozillaBrowser.h so the flag would have no effect. See bug 214884.
Comment 14 Adam Lock 2004-03-25 15:18:40 PST
Hmm, tstof might have been fixed for .NET. It might have been a missing function
in earlier versions. I stuck everything in the header simply because it means I
didn't have to change two files whenever I changed a function. I don't know why
patch was failing since it was a clean build.

Leakwise I don't believe there is anything to leak. 

I'll try and roll a new patch tomorrow, built and tested against VC98. This may
or may not update the .dsp but that's a trivial thing to fix.
Comment 15 Alexandre Trémon 2004-03-26 00:42:49 PST
It's no big deal, but in resource.h, on line 37:

#define IDD_DIALOG1                     217
Comment 16 Adam Lock 2004-03-28 03:42:29 PST
Created attachment 144947 [details] [diff] [review]
Full patch mk2

Patch defines _tstof for older VC98 and removes IDD_DIALOG1 from resource.h. 

Concerning _tstof I thought it better to define it at the top rather than
pollute the code with atof(T2CA(value)) and USES_CONVERSION macros.
Comment 17 Adam Lock 2004-03-28 23:49:02 PST
Comment on attachment 144947 [details] [diff] [review]
Full patch mk2

Requesting sr please to this implementation of the page setup dialog on the
activex contol and an API fix. r= was given but applied to previous patch by
Comment 18 Boris Zbarsky [:bz] (still a bit busy) 2004-03-29 07:38:45 PST
I'm not going to be able to get to this for at least a week, more likely two weeks.
Comment 19 Boris Zbarsky [:bz] (still a bit busy) 2004-04-10 09:55:41 PDT
Comment on attachment 144947 [details] [diff] [review]
Full patch mk2

sr=bzbarsky, I guess, but I really don't know much about win32 apis or even
some of the languages used in this patch, so I'm not sure how meaningful me
reviewing it is..
Comment 20 Adam Lock 2004-04-11 01:32:27 PDT
Comment on attachment 144947 [details] [diff] [review]
Full patch mk2

r is carried over from previous patch.

Requesting 1.7 approval. 

Patch implements page setup dialog in the ActiveX control and fixes a COM
Comment 21 Asa Dotzler [:asa] 2004-04-12 16:03:14 PDT
Comment on attachment 144947 [details] [diff] [review]
Full patch mk2

a=asa (on behalf of drivers) for checkin to 1.7
Comment 22 Adam Lock 2004-04-13 03:01:02 PDT
Fix checked into trunk and 1.7 branch

Note You need to log in before you can comment on or make changes to this bug.