Last Comment Bug 710148 - Remove MOZ_REQUIRE_CURRENT_SDK from nsPrintDialogUtil.cpp
: Remove MOZ_REQUIRE_CURRENT_SDK from nsPrintDialogUtil.cpp
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Embedding: GRE Core (show other bugs)
: Trunk
: All Windows Vista
: -- normal (vote)
: mozilla11
Assigned To: Ed Morley [:emorley]
:
Mentors:
http://mxr.mozilla.org/mozilla-centra...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-13 01:39 PST by Makoto Kato [:m_kato]
Modified: 2011-12-19 09:14 PST (History)
7 users (show)
emorley: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (16.64 KB, patch)
2011-12-14 04:16 PST, Ed Morley [:emorley]
benjamin: review+
Details | Diff | Review

Description Makoto Kato [:m_kato] 2011-12-13 01:39:48 PST
MOZ_REQUIRE_CURRENT_SDK is unused.  We should remove MOZ_REQUIRE_CURRENT_SDK related code in embedding/components/printingui/src/win/nsPrintDialogUtil.cpp.

MOZ_REQUIRE_CURRENT_SDK is used on Firefox 1?, but Gecko 1.8 and later, this isn't used.
Comment 1 Ed Morley [:emorley] 2011-12-14 04:13:39 PST
https://tbpl.mozilla.org/?tree=Try&rev=7ce27fb4cc9d
Comment 2 Ed Morley [:emorley] 2011-12-14 04:16:51 PST
Created attachment 581582 [details] [diff] [review]
Patch v1

https://tbpl.mozilla.org/?tree=Try&rev=7ce27fb4cc9d
Comment 4 Ed Morley [:emorley] 2011-12-16 05:49:02 PST
https://hg.mozilla.org/mozilla-central/rev/9e0f286f79be
Comment 5 Zack Weinberg (:zwol) 2011-12-16 08:51:01 PST
Was this change in the right direction?  It looks like we now (compile-time) unconditionally use the *old* print dialog rather than the *new* one.
Comment 6 Ed Morley [:emorley] 2011-12-17 04:24:29 PST
MOZ_REQUIRE_CURRENT_SDK wasn't defined anywhere, so the removed code shouldn't have been being compiled anyway, unless I'm missing something?

http://mxr.mozilla.org/mozilla-central/search?string=MOZ_REQUIRE_CURRENT_SDK

I'm afk until Monday, but if this was indeed the wrong way, please do back me out! :-)
Comment 7 Zack Weinberg (:zwol) 2011-12-17 08:56:27 PST
Sorry, I should have been clearer.  I'm aware it wasn't being compiled.  I'm asking whether it *should* have been compiled (possibly under some other conditional - looking at widget/src/windows, we seem to be using '#if MOZ_WINSDK_TARGETVER >= something' for similar scenarios).
Comment 8 Makoto Kato [:m_kato] 2011-12-17 20:38:55 PST
(In reply to Zack Weinberg (:zwol) from comment #7)
> Sorry, I should have been clearer.  I'm aware it wasn't being compiled.  I'm
> asking whether it *should* have been compiled (possibly under some other
> conditional - looking at widget/src/windows, we seem to be using '#if
> MOZ_WINSDK_TARGETVER >= something' for similar scenarios).

Actually, this code is broken and cannot compile due to various compile error now.

We should re-design print dialog if UX team is needed.
Comment 9 Zack Weinberg (:zwol) 2011-12-17 20:46:47 PST
(In reply to Makoto Kato from comment #8)
> 
> Actually, this code is broken and cannot compile due to various compile
> error now.
> 
> We should re-design print dialog if UX team is needed.

Thanks for the update.  In that case, I am happy with what was done here; future adjustments can happen in bug 693230 and/or bug 650957.
Comment 10 Ed Morley [:emorley] 2011-12-19 09:14:07 PST
Phew! Had me worried there for a sec! Second pair of eyes always appreciated though, so thanks for checking :-)

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