Skip to content

Commit 2e8cba1

Browse files
committed
refactor(colorcop): fix ScreenToClient bug and correct prefix checks
- Pass &magrect to ScreenToClient to avoid undefined behavior - Use _T() for prefix comparisons in OnconvertHEX - Remove stale comments and tighten cursor-handling branches - Add braces around m_MagLevel bounds checks for clarity
1 parent f738d72 commit 2e8cba1

1 file changed

Lines changed: 25 additions & 50 deletions

File tree

ColorCopDlg.cpp

Lines changed: 25 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,6 @@ void CColorCopDlg::SetupWindowRects() {
432432
::GetWindowRect(temphandle, &buttonrect);
433433
CWnd::ScreenToClient(&buttonrect);
434434

435-
436435
// Setup Q1rect for color history
437436
CWnd::GetDlgItem(IDC_Q1, &temphandle);
438437
::GetWindowRect(temphandle, &Q1rect);
@@ -476,7 +475,7 @@ void CColorCopDlg::SetupWindowRects() {
476475
HWND maghand;
477476
CWnd::GetDlgItem(IDC_MagWindow, &maghand);
478477
::GetWindowRect(maghand, &magrect);
479-
CWnd::ScreenToClient(magrect);
478+
CWnd::ScreenToClient(&magrect);
480479

481480
RECT exprect;
482481
HWND expandbutton;
@@ -737,7 +736,6 @@ void CColorCopDlg::OnconvertRGB() {
737736
}
738737

739738
// Only apply uppercase formatting when NOT in Visual C++ hex mode.
740-
// // VC+ mode requires lowercase hex, so skip TestForUpperHex() in that case.
741739
if ((m_Appflags & ModeVisualC) == 0) {
742740
TestForUpperHex();
743741
}
@@ -772,10 +770,10 @@ void CColorCopDlg::OnconvertHEX() {
772770

773771
// --- Optional prefix removal ---
774772
if (m_Appflags & OmitPound) {
775-
if ((m_Appflags & ModeHTML) && m_Hexcolor.Left(1) == "#") {
773+
if ((m_Appflags & ModeHTML) && m_Hexcolor.Left(1) == _T("#")) {
776774
m_Hexcolor.Delete(0);
777775
}
778-
if ((m_Appflags & ModeDelphi) && m_Hexcolor.Left(1) == "$") {
776+
if ((m_Appflags & ModeDelphi) && m_Hexcolor.Left(1) == _T("$")) {
779777
m_Hexcolor.Delete(0);
780778
}
781779
}
@@ -1645,52 +1643,39 @@ void CColorCopDlg::OnMouseMove(UINT nFlags, CPoint point) {
16451643
// go redraw... but don't erase or it will flicker
16461644
return;
16471645
} else {
1648-
// not magnifying oreyedropping
1646+
// not magnifying or eyedropping
16491647

16501648
CWnd* pWnd = ChildWindowFromPoint(point);
16511649
if (pWnd && pWnd->GetSafeHwnd() == m_EyeLoc.GetSafeHwnd()) {
16521650
SetCursor(m_hHandCursor);
1653-
16541651
} else if (pWnd && pWnd->GetSafeHwnd() == m_Magnifier.GetSafeHwnd()) {
16551652
// left mouse button down on the magnifier
1656-
16571653
SetCursor(m_hHandCursor);
1658-
1659-
16601654
} else if (pWnd && pWnd->GetSafeHwnd() == m_MagWindow.GetSafeHwnd()) {
1661-
// left mouse button down on magnification window
1662-
// we should get the color, because we can.
1663-
// it's in the client window.
1664-
1665-
1666-
// m_EyeLoc.SetIcon(m_hBlank);
1655+
// left mouse button down on magnification window
16671656
SetCursor(m_hEyeCursor);
1668-
1669-
} else if (pWnd && pWnd->GetSafeHwnd() == m_ColorPalette.GetSafeHwnd()) {
1657+
} else if (pWnd && pWnd->GetSafeHwnd() == m_ColorPalette.GetSafeHwnd()) {
16701658
SetCursor(m_hEyeCursor);
1671-
1672-
} else if (pWnd && pWnd->GetSafeHwnd() == m_MagPlus.GetSafeHwnd()) {
1659+
} else if (pWnd && pWnd->GetSafeHwnd() == m_MagPlus.GetSafeHwnd()) {
16731660
SetCursor(m_hHandCursor);
1674-
1675-
} else if (pWnd && pWnd->GetSafeHwnd() == m_MagMinus.GetSafeHwnd()) {
1661+
} else if (pWnd && pWnd->GetSafeHwnd() == m_MagMinus.GetSafeHwnd()) {
16761662
SetCursor(m_hHandCursor);
1677-
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q1.GetSafeHwnd()) {
1663+
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q1.GetSafeHwnd()) {
16781664
SetCursor(m_hHandCursor);
1679-
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q2.GetSafeHwnd()) {
1665+
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q2.GetSafeHwnd()) {
16801666
SetCursor(m_hHandCursor);
1681-
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q3.GetSafeHwnd()) {
1667+
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q3.GetSafeHwnd()) {
16821668
SetCursor(m_hHandCursor);
1683-
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q4.GetSafeHwnd()) {
1669+
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q4.GetSafeHwnd()) {
16841670
SetCursor(m_hHandCursor);
1685-
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q5.GetSafeHwnd()) {
1671+
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q5.GetSafeHwnd()) {
16861672
SetCursor(m_hHandCursor);
1687-
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q6.GetSafeHwnd()) {
1673+
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q6.GetSafeHwnd()) {
16881674
SetCursor(m_hHandCursor);
1689-
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q7.GetSafeHwnd()) {
1675+
} else if (pWnd && pWnd->GetSafeHwnd() == m_Q7.GetSafeHwnd()) {
16901676
SetCursor(m_hHandCursor);
1677+
}
16911678
}
1692-
}
1693-
16941679
CDialog::OnMouseMove(nFlags, point);
16951680
}
16961681

@@ -1703,7 +1688,7 @@ void CColorCopDlg::GetScreenBitmap(CPoint point) {
17031688
::DeleteObject(hZoomBitmap);
17041689
hZoomBitmap = NULL;
17051690
}
1706-
hdc = ::GetDC(NULL); // dEVICEcONEXT to the whole desktop
1691+
hdc = ::GetDC(NULL); // device context to the whole desktop
17071692

17081693
hdcMem = ::CreateCompatibleDC(hdc);
17091694
hdcZoomMem = ::CreateCompatibleDC(hdc);
@@ -1715,21 +1700,16 @@ void CColorCopDlg::GetScreenBitmap(CPoint point) {
17151700
::SelectObject(hdcZoomMem, hZoomBitmap);
17161701
::SetStretchBltMode(hdc, COLORONCOLOR);
17171702

1718-
17191703
int magwidth = 19; // default heights
17201704
int magheight = 15;
1721-
1722-
1723-
1724-
if (m_MagLevel <= 0)
1705+
if (m_MagLevel <= 0) {
17251706
m_MagLevel = 1;
1726-
else if (m_MagLevel >= 17)
1707+
} else if (m_MagLevel >= 17) {
17271708
m_MagLevel = 16;
1709+
}
17281710
magwidth = magrect.Width() / m_MagLevel;
17291711
magheight = magrect.Height() / m_MagLevel;
17301712

1731-
1732-
17331713
::BitBlt(hdcZoomMem, // destination DC
17341714
0, 0, // destination upper left (always 0, 0)
17351715
magrect.Width(), magrect.Height(), // w x h of destination
@@ -2312,8 +2292,10 @@ void CColorCopDlg::FigurePound() {
23122292
}
23132293
} else {
23142294
if (m_Appflags & ModeHTML) {
2315-
if (m_Hexcolor.Left(1) != _T("#"))
2316-
m_Hexcolor = _T("#") + m_Hexcolor; // add the # if it exsits
2295+
// if the user is switching to omit the pound, we need to add it if it doesn't exist
2296+
if (m_Hexcolor.Left(1) != _T("#")) {
2297+
m_Hexcolor = _T("#") + m_Hexcolor;
2298+
}
23172299
} else if (m_Appflags & ModeDelphi) {
23182300
if (m_Hexcolor.Left(1) != _T("$"))
23192301
m_Hexcolor = _T("$") + m_Hexcolor;
@@ -2331,7 +2313,6 @@ BOOL CAboutDlg::OnInitDialog() {
23312313
CDialog::OnInitDialog();
23322314

23332315
// setup the hyperlinks
2334-
23352316
m_link.SetLink(TRUE)
23362317
.SetTextColor(RGB(0, 0, 255))
23372318
.SetFontUnderline(TRUE)
@@ -2359,10 +2340,6 @@ void CColorCopDlg::TestForExpand() {
23592340
RECT currect;
23602341
GetWindowRect(&currect);
23612342

2362-
// usr small for both
2363-
// smWidth = lgWidth;
2364-
// smHeight = lgHeight;
2365-
23662343
if (m_Appflags & ExpandedDialog) {
23672344
currect.right = currect.left + lgWidth;
23682345
currect.bottom = currect.top + lgHeight;
@@ -2566,7 +2543,7 @@ void CColorCopDlg::OnPopupApplicationExpandeddialog() {
25662543
}
25672544

25682545
void CColorCopDlg::OnUpdatePopupApplicationExpandeddialog(CCmdUI* pCmdUI) {
2569-
pCmdUI->SetCheck(m_Appflags & ExpandedDialog);
2546+
pCmdUI->SetCheck(m_Appflags & ExpandedDialog);
25702547
}
25712548

25722549
void CColorCopDlg::OnUpdateViewHtmlhexmode(CCmdUI* pCmdUI) {
@@ -2712,7 +2689,6 @@ void CColorCopDlg::OnPopupModeVisualchex() {
27122689
OnCopytoclip();
27132690
}
27142691

2715-
27162692
void CColorCopDlg::OnPopupModeClarionhex() {
27172693
SetStatusBarText(IDS_MODE_CLARION, 1);
27182694
if (m_Appflags ^ ModeClarion) {
@@ -2729,7 +2705,6 @@ void CColorCopDlg::OnPopupModeClarionhex() {
27292705
OnCopytoclip();
27302706
}
27312707

2732-
27332708
void CColorCopDlg::OnPopupRestore() {
27342709
m_bvisible = true;
27352710

0 commit comments

Comments
 (0)