[CORE-8526] Radio buttons in calc.exe are displayed incorrectly Created: 2014-09-11  Updated: 2018-12-08

Status: Open
Project: Core ReactOS
Component/s: Wine
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Vort Assignee: Bug Zilla
Resolution: Unresolved Votes: 2
Labels: WINESYNC
Environment:

r64112
VMware Workstation 10.0.0


Attachments: PNG File ROS_Calc_Error.png     PNG File VirtualBox_ReactOS_18_12_2016_18_51_00.png     Text File calc_dialog_grouping_fix.patch     Zip Archive grouping_test_program.zip     Zip Archive grouping_test_program2.zip     Text File radio-button-grouping-fix-2.patch     Text File ros_calc_radiobuttons.patch     PNG File ros_calc_vs_win7.png     PNG File snapshot76.png     Text File user32-NextDlgItem-tests.patch    
Issue Links:
Relates
relates to CORE-12573 user32: DrawFrameControl draws bad Resolved
External URL: https://bugs.winehq.org/show_bug.cgi?id=42010

 Description   

After calc.exe started, "Dec" radio button must be selected.
But in ReactOS it is rendered as not selected.
In Windows 7 all works fine.



 Comments   
Comment by Vort [ 2015-10-05 ]

r69450
bug is still there

Comment by Katayama Hirofumi MZ [ 2016-07-31 ]

calc_dialog_grouping_fix.patch

Comment by Katayama Hirofumi MZ [ 2016-07-31 ]

Groupings of the dialog controls was wrong.
Add WS_GROUP style to radio control.
See calc_dialog_grouping_fix.patch.

Comment by Vort [ 2016-07-31 ]

But somehow Windows 7 displays this incorrect grouping correctly.

Comment by Katayama Hirofumi MZ [ 2016-08-05 ]

grouping_test_program.zip

Comment by Vort [ 2016-08-05 ]

> grouping_test_program.zip
Is this designed to simplify calc's problem?
If so, then it is not working.
Calc @r72110 still showing radio button selection in Win7, this program - is not.

Comment by Katayama Hirofumi MZ [ 2016-08-05 ]

I retried.

Comment by Vort [ 2016-08-05 ]

> I retried.
Now TEST1 is selected in Win7 and not selected in ReactOS.
TEST5 is both selected in Win7 and ReactOS.
Nice.

Comment by Katayama Hirofumi MZ [ 2016-12-15 ]

reactos/win32ss/user/user32/controls/button.c
BUTTON_CheckAutoRadioButton

Here is it!
GetNextDlgGroupItem or BUTTON_CheckAutoRadioButton contains the bug.

Comment by hbelusca [ 2016-12-15 ]

It smells like it was synced with Wine a long time ago. Can somebody have a look/comparison with latest Wine? Cc jimtabor too.

Comment by jimtabor [ 2016-12-15 ]

I have a current wine sync patch for it now. Need to commit it.

Comment by jimtabor [ 2016-12-15 ]

SS76, wine.

Comment by Katayama Hirofumi MZ [ 2016-12-16 ]

user32-NextDlgItem-tests.patch

Acording to my tests, there is no bug on GetNextDlgTabItem and GetNextDlgGroupItem.

Comment by Mark Jansen [ 2016-12-16 ]

The WS_GROUP addition seems like the correct fix.
https://msdn.microsoft.com/en-us/library/windows/desktop/ms632600(v=vs.85).aspx

Comment by Vort [ 2016-12-16 ]

Mark Jansen, it's a correct fix for Calc, but not for ReactOS.

Comment by Katayama Hirofumi MZ [ 2016-12-17 ]

radio-button-grouping-fix-2.patch

Another patch. Test me!

Comment by jimtabor [ 2016-12-17 ]

Katayama Hirofumi MZ I hope you are aware that control/button.c is wine code! It will be hard to patch ReactOS back from wine.

My guess is to send this patch to wine.

Comment by Carlo Bramix [ 2016-12-17 ]

Using WS_GROUP is not the correct way.
Calc is not using IsDialogMessage().
The MSDN link posted in a previous message explains it.

EDIT: see also:
https://bugs.winehq.org/show_bug.cgi?id=42010

Comment by Smiley [ 2016-12-17 ]

I hope we will see tests backing the changes that could be submitted for user32. A wrong fix could affect all sorts of applications.

Comment by jimtabor [ 2016-12-17 ]

Smiley user32-NextDlgItem-tests.patch is an API User32 test case.

Comment by jimtabor [ 2016-12-17 ]

Carlo Bramix Good job filing wine bug report!

Comment by Smiley [ 2016-12-17 ]

jimtabor: Oh sorry, I didn't notice it. That's great

Comment by hbelusca [ 2016-12-17 ]

There is also:
https://bugs.winehq.org/show_bug.cgi?id=16845

Comment by Katayama Hirofumi MZ [ 2016-12-18 ]

Carlo Bramix You are misunderstanding.

WM_TABSTOP
For user-created windows and modeless dialogs to work with tab stops, alter the message loop to call the IsDialogMessage function.

We don't have to work tab stops in calc. This description is not related to WS_GROUP.

Comment by jimtabor [ 2016-12-18 ]

Wow, wine'ies jump on Carlo Bramix bug report real fast. LOL!

Comment by Carlo Bramix [ 2016-12-18 ]

I applied the patch posted by Dmitry Timoshkov in WINE's bugzilla to:

reactos\win32ss\user\user32\controls\button.c

and now it seems it has solved all issues.

Comment by jimtabor [ 2016-12-18 ]

Good work Carlo Bramix ;D

Now to get them to work on my bug reports..... :/

Comment by jimtabor [ 2016-12-19 ]

Test result from wine:

wine ./user32_apitest.exe NextDlgItem
preloader: Warning: failed to reserve range 00010000-00110000
preloader: Warning: failed to reserve range 00010000-00110000
preloader: Warning: failed to reserve range 00010000-00110000
preloader: Warning: failed to reserve range 00010000-00110000
fixme:service:scmdatabase_autostart_services Auto-start service L"escSrv" failed to start: 2
preloader: Warning: failed to reserve range 00010000-00110000
preloader: Warning: failed to reserve range 00010000-00110000
preloader: Warning: failed to reserve range 00010000-00110000
preloader: Warning: failed to reserve range 00010000-00110000
err:server:find_window_to_repaint PAINT_INTERNAL clr 1
err:msg:DispatchMessageW WM_PAINT W!
NextDlgItem.c:350: Test failed:

NextDlgItem: 123 tests executed (0 marked as todo, 1 failure), 0 skipped.

Forgot the button patch.....

Comment by jimtabor [ 2016-12-19 ]

With the wine button patch wine passes the test.

Comment by jimtabor [ 2016-12-19 ]

Katayama Hirofumi MZ Committed the test case in revision 73468.

Comment by jimtabor [ 2016-12-19 ]

Now wait for wine staging.

Comment by Jedi-to-be [ 2016-12-24 ]


Status: STAGED

Comment by Mark Jansen [ 2017-08-18 ]

Can we include calc.exe in the next staging round?
AmineKhaldi

Comment by hbelusca [ 2017-08-18 ]

What do you mean Mark Jansen? If I understand correctly, our calc.exe is the original one by Carlo Bramix, while wine's one may be just a fork.

Comment by Mark Jansen [ 2017-08-18 ]

Oh i thought this was a wine-sync.
nvm then

Comment by DougLyons [ 2018-06-23 ]

The fix for this has been in wine-staging for over a year. Here is the link:

https://bugs.winehq.org/show_bug.cgi?id=42010

I have adapted it and added #ifdef's for ReactOS. It would be very nice to see this problem fixed for our calculator.

Comment by extravert34 [ 2018-07-29 ]

looks like this is fixed

Comment by DougLyons [ 2018-07-29 ]

extravert34, my testing indicates that this is not fixed. I just created a new virtual machine from the latest ISO downloaded from the ReactOS website (10-380) and installed this in VMware Player 12 taking all defaults. Then I opened Calculator and from the menu bar selected "View | Scientific". Then I captured the attached screen shot that shows that none of the radio buttons are selected in either of the panels where they should have selections. In addition if I try and select any of the "Hex-Dec-Oct-Bin" buttons, their radio button will not stay highlighted. Can you tell me how you tested?

Comment by extravert34 [ 2018-07-30 ]

Tested latest builds from master. This error happens only in gcc build, msvc is ok.

Comment by DougLyons [ 2018-07-30 ]

extravert34 that is a great finding. Thank you for sharing it. I have not been able to build with msvc yet because of 3 errors that I have not been able to fix. Can you try the attached patch and see if msvc still works OK with it? Thanks.

Comment by hbelusca [ 2018-08-21 ]

It seems that the radio buttons should be groupped using the WS_GROUP for Hex, (and no WS_GROUP for Oct, Dec, Bin), and again with WS_GROUP for Deg, (and no WS_GROUP for Rad and Grades) for example...

Comment by DougLyons [ 2018-08-21 ]

hbelusca, please see Carlo Bramix's two comments above about this and consider that the calculator works just fine in Windows XP and Server 2003. ReactOS should work as well as XP and WS2K3. I have attached a Wine patch that works. Thanks.

Comment by Carlo Bramix [ 2018-11-28 ]

This bug has been resolved in WINE:

https://source.winehq.org/git/wine.git/commit/96d0af52eb0d14084397647b974c5efebb59d0f0

A sync with WINE source codes will close this issue also on ReactOS.

Comment by justincase [ 2018-11-28 ]

Is it part of 1.12?

cf https://github.com/reactos/reactos/pull/733

Comment by DougLyons [ 2018-11-29 ]

justincase, this is not part of the Calculator update to version 1.12 PR#733 patch.

Also this WINE commit is a part of the fix:

https://source.winehq.org/git/wine.git/commit/b1b8fb77be5dd9a8754b04b2ef9f703bbe393d59

This is for the keyboard interface. I believe that these two WINE commits match my last posted patch without the #ifdef's for ReactOS.

Now it should just be a matter of waiting for the next WINE sync to have this fixed.

Edit: Sorry I was mistaken. It appears that the code in this second commit affects win32ss/user/user32/windows/dialog.c which is not WINE-synced, so this will need to be done separately and could be done any time now. Thanks.

Comment by Carlo Bramix [ 2018-12-08 ]

The fix is now in master.

https://www.winehq.org/announce/4.0-rc1

Generated at Wed Dec 19 05:14:12 UTC 2018 using Jira 7.12.3#712004-sha1:5ef91d760d7124da5ebec5c16a948a4a807698df.