Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ttk style.map function incorrectly handles the default state for element options. #86494

Closed
patthoyts mannequin opened this issue Nov 12, 2020 · 14 comments
Closed

ttk style.map function incorrectly handles the default state for element options. #86494

patthoyts mannequin opened this issue Nov 12, 2020 · 14 comments
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes topic-tkinter type-bug An unexpected behavior, bug, or error

Comments

@patthoyts
Copy link
Mannequin

patthoyts mannequin commented Nov 12, 2020

BPO 42328
Nosy @terryjreedy, @db3l, @serhiy-storchaka, @patthoyts, @miss-islington, @patthoyts
PRs
  • bpo-42328: ttk fix handling of default states in style map #23241
  • bpo-42328: Fix tkinter.ttk.Style.map(). #23300
  • [3.9] bpo-42328: Fix tkinter.ttk.Style.map(). (GH-23300) #23470
  • [3.8] bpo-42328: Fix tkinter.ttk.Style.map(). (GH-23300) #23471
  • bpo-42328: Skip some tests with themes vista and xpnative on Windows 7 #23612
  • [3.9] bpo-42328: Skip some tests with themes vista and xpnative on Windows 7 (GH-23612) #23624
  • [3.8] bpo-42328: Skip some tests with themes vista and xpnative on Windows 7 (GH-23612) #23625
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2020-12-15.16:07:46.055>
    created_at = <Date 2020-11-12.00:27:54.034>
    labels = ['3.8', 'type-bug', 'expert-tkinter', '3.9', '3.10']
    title = 'ttk style.map function incorrectly handles the default state for element options.'
    updated_at = <Date 2020-12-15.16:07:46.054>
    user = 'https://github.com/patthoyts'

    bugs.python.org fields:

    activity = <Date 2020-12-15.16:07:46.054>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-12-15.16:07:46.055>
    closer = 'serhiy.storchaka'
    components = ['Tkinter']
    creation = <Date 2020-11-12.00:27:54.034>
    creator = 'patthoyts'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 42328
    keywords = ['patch']
    message_count = 14.0
    messages = ['380798', '380967', '380992', '380994', '380995', '381013', '381636', '381652', '381653', '381964', '382298', '382386', '382389', '382390']
    nosy_count = 6.0
    nosy_names = ['terry.reedy', 'db3l', 'serhiy.storchaka', 'Pat Thoyts', 'miss-islington', 'patthoyts']
    pr_nums = ['23241', '23300', '23470', '23471', '23612', '23624', '23625']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue42328'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    @patthoyts
    Copy link
    Mannequin Author

    patthoyts mannequin commented Nov 12, 2020

    When cloning a ttk style it is useful to copy an existing style and make changes. We can copy the configuration and layout using:

        style.layout('Custom.TEntry', **style.layout('TEntry'))
        style.configure('Custom.TEntry', **style.configure('TEntry))

    However, doing this with style.map can result in an exception. An example of this occurs for any style that has a defined default state in the map eg the TNotebook.Tab in the clam theme:

        >>> style.map('TNotebook.Tab','background')
        [('selected', '#dcdad5'), ('#bab5ab',)]

    However, calling Tk directly:

        >>> style.tk.call(style._name,"map","TNotebook.Tab","-background")
        (<StateSpec object: 'selected'>, '#dcdad5', <StateSpec object: ''>, '#bab5ab')

    The second pair is defining the default state ('') to use color #bab5ab but this is being mangled by the code that converts this into pythons response.

    The culprit is ttk._list_from_statespec which treats the statespec with the empty string as None and drops it and then returns the value in place of the state and then puts in an empty value.

    @patthoyts patthoyts mannequin added 3.8 only security fixes topic-tkinter type-bug An unexpected behavior, bug, or error labels Nov 12, 2020
    @terryjreedy
    Copy link
    Member

    You have 2 user names, both nosy. You logged in with the lower-case name, hence the Author listing above. However, your CLA was registered with your uppercase (older?) login name. (If this is not what you intended, please write ewa at ython.org and explain.) Hence your name (login name) is not followed by the CLA * marker. This was a bit confusing until I decyphered the situaiton.

    @serhiy-storchaka
    Copy link
    Member

    Sorry, I did not understand how did you get an empty state? Could you please provide complete reproducible example?

    @patthoyts
    Copy link
    Mannequin Author

    patthoyts mannequin commented Nov 14, 2020

    So if you look at the clamTheme.tcl file you can see the definition of the map for the TNotebook.Tab style looks like the following:

    ttk::style map TNotebook.Tab \
      -padding [list selected {6 4 6 2}] \
      -background [list selected $colors(-frame) {} $colors(-darker)] \
      -lightcolor [list selected $colors(-lighter) {} $colors(-dark)] \
      ;
    

    The vista theme uses these too on Windows.

    So calling this from script we can see the resulting empty elements in tcl:

    % ttk::style map TNotebook.Tab
    -lightcolor {selected #eeebe7 {} #cfcdc8} -padding {selected {6 4 6 2}} -background {selected #dcdad5 {} #bab5ab}
    

    As I put in the bug, this gets mistranslated in python with the value for that state map element getting put into the first element.

    The simplest demonstration is that the following raises an exception:

        import tkinter as tk
        import tkinter.ttk as ttk
        style = ttk.Style()
        style.theme_use('clam')
        style.map('Custom.TNotebook.Tab', **style.map('TNotebook.Tab'))

    @terryjreedy
    Copy link
    Member

    Verified
    Traceback (most recent call last):
      File "F:\Python\a\tem4.py", line 5, in <module>
        style.map('Custom.TNotebook.Tab', **style.map('TNotebook.Tab'))
      File "C:\Program Files\Python310\lib\tkinter\ttk.py", line 403, in map
        self.tk.call(self._name, "map", style, *_format_mapdict(kw)),
      File "C:\Program Files\Python310\lib\tkinter\ttk.py", line 111, in _format_mapdict
        _format_optvalue(_mapdict_values(value), script)))
      File "C:\Program Files\Python310\lib\tkinter\ttk.py", line 85, in _mapdict_values
        state[0] # raise IndexError if empty
    IndexError: list index out of range
    >>> 

    PS. Pat, please don't indent code that someone might reasonably copy and paste to run.

    @serhiy-storchaka
    Copy link
    Member

    Thank you for your examples Pat. Now it looks clearer to me.

    Semantically this data in Tk is a sequence of pairs: states (which can be a single word or several words or empty) and default value.

    >>> from tkinter import ttk
    >>> style = ttk.Style()
    >>> style.theme_use('clam')
    >>> style.tk.eval(f'{style._name} map TCombobox -fieldbackground')
    '{readonly focus} #4a6984 readonly #dcdad5'
    >>> style.tk.eval(f'{style._name} map TNotebook.Tab -background')
    'selected #dcdad5 {} #bab5ab'

    Internally states represented in Tk as the StateSpec object. When automatically convert to Python it became the Tcl_Obj object with typename 'StateSpec'. Without postprocessing.

    >>> style.tk.call(style._name, 'map', 'TCombobox', '-fieldbackground')
    (<StateSpec object: 'readonly focus'>, '#4a6984', <StateSpec object: 'readonly'>, '#dcdad5')
    >>> style.tk.call(style._name, 'map', 'TNotebook.Tab', '-background')
    (<StateSpec object: 'selected'>, '#dcdad5', <StateSpec object: ''>, '#bab5ab')

    Style.map() does postprocessing. It converts a sequence (with even items number) to a list of tuples. The last item of a tuple is the default value, and the rest are items of the StateSpec object.

    >>> style.map('TCombobox', 'fieldbackground')
    [('readonly', 'focus', '#4a6984'), ('readonly', '#dcdad5')]
    >>> style.map('TNotebook.Tab', 'background')
    [('selected', '#dcdad5'), ('#bab5ab',)]

    But when set tkinter.wantobjects = 0 before running this example the result will be different, because StateSpec objects will be automatically represented as strings (it matches the behavior of initial versions of Tkinter):

    >>> style.tk.call(style._name, 'map', 'TCombobox', '-fieldbackground')
    '{readonly focus} #4a6984 readonly #dcdad5'
    >>> style.tk.call(style._name, 'map', 'TNotebook.Tab', '-background')
    'selected #dcdad5 {} #bab5ab'
    >>> style.map('TCombobox', 'fieldbackground')
    [('readonly focus', '#4a6984'), ('readonly', '#dcdad5')]
    >>> style.map('TNotebook.Tab', 'background')
    [('selected', '#dcdad5'), ('', '#bab5ab')]

    The main problem is in representing an empty StateSpec. As every string in Python contains an empty string, {} can represent an empty sequence and a sequence containing single empty string. In Python, it can be no items before default value (like in ('#bab5ab',)) or a single item containing an empty string (like in ('', '#bab5ab')).

    The former representation (an empty sequence) is default for the style.map() output, but it is rejected as the style.map() input (see state[0] in _mapdict_values). There are two ways to fix it: either change the output of style.map() (what PR 23241 does) or change the validation of the input. I think that the latter solution can be backported, but the former can be used only in the future Python version.

    @serhiy-storchaka
    Copy link
    Member

    New changeset dd844a2 by Serhiy Storchaka in branch 'master':
    bpo-42328: Fix tkinter.ttk.Style.map(). (GH-23300)
    dd844a2

    @miss-islington
    Copy link
    Contributor

    New changeset 3e53301 by Miss Islington (bot) in branch '3.9':
    bpo-42328: Fix tkinter.ttk.Style.map(). (GH-23300)
    3e53301

    @miss-islington
    Copy link
    Contributor

    New changeset ad49526 by Miss Islington (bot) in branch '3.8':
    bpo-42328: Fix tkinter.ttk.Style.map(). (GH-23300)
    ad49526

    @db3l
    Copy link
    Contributor

    db3l commented Nov 27, 2020

    This change to the 3.8 branch appears to be consistently failing on the Windows 7 buildbot (first failing build at https://buildbot.python.org/all/#/builders/270/builds/126).

    It's all failures in the new test_configure_custom_copy and test_map_custom_copy tests, such as:

    FAIL: test_configure_custom_copy (tkinter.test.test_ttk.test_style.StyleTest) (theme='vista', name='.')
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "D:\cygwin\home\db3l\buildarea\3.8.bolen-windows7\build\lib\tkinter\test\test_ttk\test_style.py", line 140, in test_configure_custom_copy
        self.assertEqual(style.configure(newname), None)
    AssertionError: {'foreground': 'SystemWindowText', 'selec[233 chars]bar'} != None

    and

    ======================================================================
    FAIL: test_map_custom_copy (tkinter.test.test_ttk.test_style.StyleTest) (theme='vista', name='.')
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "D:\cygwin\home\db3l\buildarea\3.8.bolen-windows7\build\lib\tkinter\test\test_ttk\test_style.py", line 162, in test_map_custom_copy
        self.assertEqual(style.map(newname), {})
    AssertionError: {'foreground': [('disabled', 'SystemGrayTe[34 chars]1')]} != {}
    + {}
    - {'embossed': [('disabled', '1')],
    -  'foreground': [('disabled', 'SystemGrayText')]}

    Since it seems related to themes, I should mention that the buildbot is running with a Windows 7 classic (non-Aero) theme with all appearance effects disabled (the "best performance" option). So I'm not sure if this is an issue with the code changes, or behavior due to the host environment not anticipated by the tests.

    @serhiy-storchaka
    Copy link
    Member

    Thank you for your report David. I have not Windows 7 and does not know how to reproduce this on Windows 10 so will just skip tests for specific themes on Windows 7.

    @serhiy-storchaka
    Copy link
    Member

    New changeset f3c3ea9 by Serhiy Storchaka in branch 'master':
    bpo-42328: Skip some tests with themes vista and xpnative on Windows 7 (GH-23612)
    f3c3ea9

    @miss-islington
    Copy link
    Contributor

    New changeset 12d2306 by Miss Islington (bot) in branch '3.8':
    bpo-42328: Skip some tests with themes vista and xpnative on Windows 7 (GH-23612)
    12d2306

    @miss-islington
    Copy link
    Contributor

    New changeset ae67db6 by Miss Islington (bot) in branch '3.9':
    bpo-42328: Skip some tests with themes vista and xpnative on Windows 7 (GH-23612)
    ae67db6

    @serhiy-storchaka serhiy-storchaka added 3.9 only security fixes 3.10 only security fixes labels Dec 15, 2020
    @serhiy-storchaka serhiy-storchaka added 3.9 only security fixes 3.10 only security fixes labels Dec 15, 2020
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes topic-tkinter type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants