Jon Griffiths wrote:
Hi,
A fairly large update to the tab control. This makes it work as per native in my app.
Regards, Jon
+dlls/comctl32/tab.c dlls/comctl32/tests/tab.c Items can be variable sized; use an accessor to retrieve them Send WM_NOTIFYFORMAT/WM_QUERYUISTATE to match native messages
There is no point in sending WM_QUERYUISTATE. I believe it is only really used for the toolbar control, but the native comctl32 has a common framework that means it is sent for all controls.
Only create a scroll control when/if we need one (like native) Dump tab extra data when tracing Combine all Unicode/Ascii calls together Pass infoPtr around instead of fetching it multiple times, store hwnd and focus state in it too. Calculate correct tab widths when TCS_FIXEDWIDTH is not given Code simplifications/const correctness/formatting fixes Add some new combinations to tests for correct tab sizes
I haven't looked at the rest of the changes yet and I won't have a chance until Sunday, but the changelog looks sane enough. Is it possible to split the patch up to make it more easily reviewable?
Rob
Hi,
There is no point in sending WM_QUERYUISTATE. I believe it is only really used for the toolbar control, but the native comctl32 has a common framework that means it is sent for all controls.
Sure, it helps when diffing the traces of native and builtin though. By sending this and creating the scroll window later I get the exact same traces and window handles as native. That makes it a lot easier to find differences later on. Its also concievable (though unlikely) that the parent may use the message to determine the control is being created.
I haven't looked at the rest of the changes yet and I won't have a chance until Sunday, but the changelog looks sane enough. Is it possible to split the patch up to make it more easily reviewable?
Possible, but a lot of work. the change to pass infoPtr around hits every function definition and function call, and the change to use an accessor hits every access to the infoPtr->items array. I'm happy if Alexandre doesn't commit until you've had a chance to review it though.
Cheers, Jon
===== "Don't wait for the seas to part, or messiahs to come; Don't you sit around and waste this chance..." - Live
jon_p_griffiths@yahoo.com
_______________________________ Do you Yahoo!? Declare Yourself - Register online to vote today! http://vote.yahoo.com
Jon Griffiths wrote:
Hi,
There is no point in sending WM_QUERYUISTATE. I believe it is only really used for the toolbar control, but the native comctl32 has a common framework that means it is sent for all controls.
Sure, it helps when diffing the traces of native and builtin though. By sending this and creating the scroll window later I get the exact same traces and window handles as native. That makes it a lot easier to find differences later on. Its also concievable (though unlikely) that the parent may use the message to determine the control is being created.
Ok, there is no harm in having it in. Just for the record it is used to emulate the keyboard cues in Windows 2000+ menus.
I haven't looked at the rest of the changes yet and I won't have a chance until Sunday, but the changelog looks sane enough. Is it possible to split the patch up to make it more easily reviewable?
Possible, but a lot of work. the change to pass infoPtr around hits every function definition and function call, and the change to use an accessor hits every access to the infoPtr->items array. I'm happy if Alexandre doesn't commit until you've had a chance to review it though.
Ok. Deal.
Rob
Hi,
It would be really nice to separate formatting changes from other changes. It's hard to see what's got changed and what not.
Agreed, see my reply to Robert on that.
"Only create a scroll control when/if we need one (like native)"
Nothing got changed. It's being created when needed, and hidden when not longed used. Is there something else I missed?
Yes, we don't need a scroll control if the window is sized to 0,0 when items are added.
Combine all Unicode/Ascii calls togetherI'm not sure here, but should it be based on the unicode state of
the
whole control rather which message got sent to it?
This refers to processing the messages using the same code. e.g. between TAB_InsertItemA/W only 2 lines were different - the debug trace and the line that sets the text pointer. Its better to share the code so that we only have one place where the items array is realloced on insert. Its also a lot smaller (see also get/set item A/W).
I can see how you can send ASCII message first and then send UNICODE message which will screw things up.
I don't know what you mean. Can you give an example?
If this is the case we need to store pointers in array of pointers.
You misunderstand. Items are variable sized per _control_, not per _item_. So array indexing is better, its just that the size of the indexed item changes for each control depending on its data size.
You missed insert items with text part inside create_tabcontrol. Besides as is it's failing for me. When I change text of the first tab "Tab 123" it's changing returned width.
Its measuring the size of the text, so _of course_ it will fail if you change the text. If it fails on a standard windows setup I can rework the test to calculate the text width manually for the comparason. But I don't have a win system setup here to test ATM. The test works here with builtin and native comctl32.
The reason why I didn't test this is because the width of the tab depends on it's text width. Which is kind of hard to test
(different
fonts, different font sizes etc.).
If the tab doesn't have text and/or icons and tabs are not fixed width the size is currently wrong. The patch fixes those cases (that my app depends on) and doesn't break any other case that I tested. Can you provide a case that I broke that wasn't already broken?
Cheers, Jon
===== "Don't wait for the seas to part, or messiahs to come; Don't you sit around and waste this chance..." - Live
jon_p_griffiths@yahoo.com
__________________________________ Do you Yahoo!? Yahoo! Mail is new and improved - Check it out! http://promotions.yahoo.com/new_mail
Jon Griffiths wrote:
Combine all Unicode/Ascii calls together
I'm not sure here, but should it be based on the unicode state of
the
whole control rather which message got sent to it?
This refers to processing the messages using the same code. e.g. between TAB_InsertItemA/W only 2 lines were different - the debug trace and the line that sets the text pointer. Its better to share the code so that we only have one place where the items array is realloced on insert. Its also a lot smaller (see also get/set item A/W).
I'm not sure what Vitaliy sees wrong with the new code, but it is easier to maintain and simpler. Vitaliy is right that every common control should send notifications based on the Unicode format of the control rather than the message that got sent to it, but the mentioned functions don't send any notifications.
You missed insert items with text part inside create_tabcontrol. Besides as is it's failing for me. When I change text of the first tab "Tab 123" it's changing returned width.
Its measuring the size of the text, so _of course_ it will fail if you change the text. If it fails on a standard windows setup I can rework the test to calculate the text width manually for the comparason. But I don't have a win system setup here to test ATM. The test works here with builtin and native comctl32.
The test fails here also without changing anything. Does this mean that the magic number 54 isn't correct?
tab.c:89: Test failed: Expected width [54] got [61] tab.c:95: Test failed: Expected [54,20] got [61,20] tab.c:95: Test failed: Expected [54,1] got [61,1] tab.c:95: Test failed: Expected [75,30] got [82,30] tab.c:95: Test failed: Expected [75,20] got [82,20] tab.c:95: Test failed: Expected [75,1] got [82,1]
Rob
I can see how you can send ASCII message first and then send UNICODE message which will screw things up.
I don't know what you mean. Can you give an example?
Sorry, I haven't paid attention to how we store text. So it shouldn't be an issue.
You missed insert items with text part inside create_tabcontrol. Besides as is it's failing for me. When I change text of the first tab "Tab 123" it's changing returned width.
Its measuring the size of the text, so _of course_ it will fail if you change the text. If it fails on a standard windows setup I can rework the test to calculate the text width manually for the comparason. But I don't have a win system setup here to test ATM. The test works here with builtin and native comctl32.
What I was talking about as in the tests/tab.c function create_tabcontrol besides creating tab control itself, inserts three tabs with text. With first tab being "Tab 123". Even when you not specify "TCIF_TEXT" tab still has that text in it, it's just not displayed.
As-is (no changes) your test fails for me. Try setting some common font with known size. BTW I tried that and it didn't work right. So we have some other problems here.
The reason why I didn't test this is because the width of the tab depends on it's text width. Which is kind of hard to test (different fonts, different font sizes etc.).
If the tab doesn't have text and/or icons and tabs are not fixed width the size is currently wrong. The patch fixes those cases (that my app depends on) and doesn't break any other case that I tested. Can you provide a case that I broke that wasn't already broken?
No, I don't have such a case. The problem is it's not exactly right. And some of your tests fail for me. I don't think AJ will except you patch if it doesn't pass tests.
-
Jon Griffiths -
Robert Shearman -
Vitaliy Margolen