Closed
Bug 144216
Opened 22 years ago
Closed 22 years ago
mozilla 1.0rc2 doesn't build with --enable-toolkit-qt
Categories
(Core Graveyard :: GFX, defect, P3)
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 178987
People
(Reporter: bero, Assigned: bero)
References
Details
Attachments
(1 file)
(deleted),
patch
|
roland.mainz
:
review+
|
Details | Diff | Splinter Review |
SSIA - Attaching a patch.
Assignee | ||
Comment 1•22 years ago
|
||
This makes it compile with Qt 2.3.x; it's still a no-go with Qt 3.x because
Mozilla hasn't been adapted to the new charset yet.
Updated•22 years ago
|
wow this is long
these are things i'd like changed if it weren't for the fact that you're fixing
build bustage by cloning code.
[re MAKE_PR_BOOL] i'm pretty sure you can use PRBool() to cast things, but
in general you should just be using if (cond) or if (!cond) instead of
comparisons to PR_TRUE/PR_FALSE so it shouldn't matter. if you're going to use
something, might i suggest !!expr instead of (expr)?PR_TRUE:PR_FALSE?
+// then it will never be delete, so this class takes care of that.
be delete_d_
+ /* Does this device allow to set/change the paper size ? */
allow _??_ to/g
+ /* Set number of orientation records and the records itself */
grm: records <-> itself. choose from 'record itself' or 'records themselves'
+ mPrefs->SetCharPref(nsPrintfCString(256, PRINTERFEATURES_PREF ".%s.%s",
mPrinterName.get(), tagname).get(), value);
what happens if you have a 300 character long printer name?
- NS_ASSERTION(aPS, "Must have a PrintSettings!");
+ DO_PR_DEBUG_LOG(("nsDeviceContextSpecQT::DisplayXPDialog()\n"));
grm: + NS_ASSERTION(aPS, "Must have a print settings!");
PrintSettings was ok because it was a type, 'print settings' isn't because it
isn't correct English.
+ /* printer names for the PostScript module alwas start with
sp: alwas
+ if (aCount)
+ *aCount = 0;
+ else
+ return NS_ERROR_NULL_POINTER;
else before return (personal nit)/g
if you had written:
+ if (!aCount)
+ return NS_ERROR_NULL_POINTER;
+ else
+ *aCount = 0;
then an sr would have complained about else after return.
consider this instead:
+ if (!aCount)
+ return NS_ERROR_NULL_POINTER;
+ *aCount = 0;
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
note:i think people are leaning towards NS_ENSURE_TRUE(NS_SUCCEEDED(rv), rv);/g
+ nsresult rv = GlobalPrinters::GetInstance()->InitializeGlobalPrinters();
...
+ PRInt32 numPrinters = GlobalPrinters::GetInstance()->GetNumPrinters();
is there some important reason not to cache GetInstance()?/g
+ nsMemory::Free(array[i]);
+ nsMemory::Free(array);
warning: msvc6 free (and i have don't want to think about what nsMemory::Free
actually does) will complain if you don't cast its param to void*.
+ XpuOrientationRec *curr = &olist[i];
+ printerFeatures.SetOrientationRecord(i, curr->orientation);
why not:
+ printerFeatures.SetOrientationRecord(i,
((XpuOrientationRec)olist[i]).orientation);
your code uses both
+ }
+ else {
and
+ } else {
i prefer the latter or consistency.
+ return NS_OK;
+ }
+ else
else after return. lose it :)
+ /* fixme: We simply ignore the |aPrinter| argument here
mozilla style includes prefixing things like this with XXX
+ mGlobalPrinterList->AppendString(
+ nsString(NS_ConvertASCIItoUCS2(NS_POSTSCRIPT_DRIVER_NAME)) +
+ nsString(NS_ConvertASCIItoUCS2(name)));
this looks like bad ns*String foo, but it's too late for me to spend time
checking.
+GlobalPrinters::GetDefaultPrinterName(PRUnichar **aDefaultPrinterName)
..
+ if (NS_FAILED(rv)) {
+ return;
+ }
..
+ if (GlobalPrinters::GetInstance()->GetNumPrinters() == 0)
+ return;
personal nit, be consistent about wrapping single block returns. presonally I
prefer the latter (or consistency)
notations /g = global nit. grm: = grammar. sp: = spelling
anyways, i'm willing to check this in [to trunk] (if you want) if cls/blizzard
take a moment to comment. for branch you'd need to send an additional email to
drivers@mozilla.org explaining that this lets people build mozilla against qt.
Assignee: kmcclusk → bero
Blocks: qt
Comment 3•22 years ago
|
||
Nice... I wish we would get bug 130857 ("Create unified version of
nsDeviceContextSpec{GTK|Qt|Xlib}.{cpp|h} stuff in gfx/src/unixshared/")
implemented instead but this one is enougth to get the Qt port back to live...
:)
Thanks for the work...
----
timeless wrote:
> these are things i'd like changed if it weren't for the fact that you're
> fixing build bustage by cloning code.
That's what makes the difference between nsDeviceContextGTK <-->
nsDeviceContextQt <--> nsDeviceContextXlib - always the same code but different
class names.
That's why I filed bug 130857 to get a unified version of the code (for
GTK/Qt/Xlib/BeOS) to avoid that one platform get's patched without updating the
others, too...
Assignee | ||
Comment 4•22 years ago
|
||
Unfortunately the patch isn't sufficient to make it actually work yet... It compiles (that's when I sent the patch - the rest of the build took some more hours), but then barfs on startup: GFX: dpi=75 t2p=0.0526316 p2t=19 depth=16 JCG: eBorderStyle_close isn't handled yet... please fix me
fwiw eBorderStyle_close isn't handled in gtk either.
my bet is we core or exit, use your favorite debugger, good luck.
note that getting it to build satisfies 'fixing build bustage' and would enable
others (eg hwaara) to try to help out, so it's still worth looking into putting
on trunk. (i probably wouldn't ask drivers to put it on branch)
Assignee | ||
Comment 6•22 years ago
|
||
Correction: It doesn't crash, it just hangs forever after displaying the
eBorderStyle_close message.
My first guess is it waits for user input without bothering to show the dialog
it's waiting for first, probably it forgets to enter the Qt event loop.
Last couple of lines from ltrace:
_ZNK24nsASingleFragmentCString19GetReadableFragmentER18nsReadableFragmentIcE17nsFragmentRequestj(0xbfffe160,
0xbfffe060, 1, 0, 0) = 0x4040e8e0
_ZNK24nsASingleFragmentCString19GetReadableFragmentER18nsReadableFragmentIcE17nsFragmentRequestj(0xbfffe160,
0xbfffe060, 3, 0, 0xbfffe160) = 0
_ZNK24nsASingleFragmentCString19GetReadableFragmentER18nsReadableFragmentIcE17nsFragmentRequestj(0xbfffe150,
0xbfffe060, 1, 0, 0) = 0x4040e8e5
_ZNK24nsASingleFragmentCString19GetReadableFragmentER18nsReadableFragmentIcE17nsFragmentRequestj(0xbfffe150,
0xbfffe060, 3, 0, 0) = 0
--- 0 (Real-time signal 0) ---
breakpointed at 0x42028828 (?)
breakpointed at 0x40444f57 (?)
_ZNK24nsASingleFragmentCString19GetReadableFragmentER18nsReadableFragmentIcE17nsFragmentRequestj
== nsASingleFragmentCString::GetReadableFragment(nsReadableFragment<char>&,
nsFragmentRequest, unsigned) const
Don't have the time to debug this any further ATM, if nobody else picks it up
I'll take a closer look later.
much better, i don't have time not but that bug's filed (component:string,
assignee: jag/scc) you should be able to find it.
Comment 8•22 years ago
|
||
The code in widget/src/qt/ may need an update, maybe the XP timer changes were
never ported to Qt.
Please get this bug fixed first (build bustage) and file a new bug for the new
issue, assign it to me and CC: timeless...
Assignee | ||
Comment 9•22 years ago
|
||
done. Bug 144428.
Comment 10•22 years ago
|
||
Comment on attachment 83375 [details] [diff] [review]
Fix build problems with --enable-toolkit-qt
r=Roland.Mainz@informatik.med.uni-giessen.de - let's get it "in" into trunk
first
Attachment #83375 -
Flags: review+
Updated•22 years ago
|
Priority: -- → P3
Comment 11•22 years ago
|
||
*** Bug 130725 has been marked as a duplicate of this bug. ***
Comment 12•22 years ago
|
||
timeless, can you check this in? I don't want to think about your comment 2...
this is ports code, so rs=blizzard, and we have an r=
Comment 13•22 years ago
|
||
This patch fails for the current mozilla CVS as well as 1.1b. Bug 144469 also
contains this comment. This should have a re-look to fix up for the current CVS,
quite a large chunk fails.
Comment 14•22 years ago
|
||
*** This bug has been marked as a duplicate of 178987 ***
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → DUPLICATE
Updated•16 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•