Edgewall Software
Modify

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#11813 closed enhancement (fixed)

Move default handler preference to another preference tab

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.1.3
Component: general Version:
Severity: normal Keywords: preferences
Cc:
Release Notes:
  • Improved layout of elements on the Advanced, Date & Time and Language preferences panels.
  • Combined Date & Time and Language preference panels as Localization.
  • Moved default_handler from the General to the User Interface preferences panel.
  • Moved localization settings on the Basic Settings admin page to their own fieldset.
  • Renamed Date relative/absolute format label to Time format.
API Changes:

Description

It was discussed in comment:28:ticket:11519 that the General tab might not be the best location for the Default Handler preference. We'll consider moving it to a different, or new, tab.

Attachments (4)

DateSelectorsOnSameLine.png (34.3 KB ) - added by Ryan J Ollos 4 years ago.
NewBasicSettings.png (41.6 KB ) - added by Ryan J Ollos 4 years ago.
NewPrefsLocalization.png (73.8 KB ) - added by Ryan J Ollos 4 years ago.
NewPrefsUserInterface.png (40.5 KB ) - added by Ryan J Ollos 4 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 4 years ago by Ryan J Ollos

Some proposed changes in log:rjollos.git:t11813. Any thoughts on the new Navigation preferences tab?

[e2fa04e6/rjollos.git] is unrelated and could be backported to 1.0-stable.

comment:2 Changed 4 years ago by Ryan J Ollos

Release Notes: modified (diff)

[e2fa04e6/rjollos.git] applied to 1.0-stable in [13262], merged to trunk in [13263]. I'll wait a bit longer to see if there is feedback on primary changes for this ticket: [c18de801/rjollos.git].

comment:3 Changed 4 years ago by Peter Suter

Replying to rjollos:

Can we foresee any additional entries for Keyboard shortcuts?

One could look to plugins for inspiration, e.g.:

Neither has preferences at the moment, but including them in core might well lead to additional checkboxes. Navigation would not fit well for those though.

User interface and Navigation both seems acceptable to me for the default handler.

Navigation seems good for anything related to the nav bars. It seems acceptable for most of the existing TracAccessibility keyboard shortcuts (except e for editing.)

Accessibility might also fit those, but maybe not much else.

At the moment I have maybe a slight preference for keeping Keyboard shortcuts and moving default handler to User interface, but if Navigation is chosen instead that's fine as well.

comment:4 Changed 4 years ago by Ryan J Ollos

Another possibility for combining preferences panels is to locate Date & Time and Language together on a panel title Localization.

comment:5 Changed 4 years ago by Ryan J Ollos

The change in indentation of two paragraph texts in [13262] results in noise in the message extraction. I tried the suggestions in comment:13:ticket:11127. HTML comment markers are only effective if wrapped in an i18n:msg:

  • trac/prefs/templates/prefs_advanced.html

    diff --git a/trac/prefs/templates/prefs_advanced.html b/trac/prefs/templates/pre
    index 82fb2af..583d4b8 100644
    a b  
    1313    "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
    1414<html xmlns="http://www.w3.org/1999/xhtml"
    1515      xmlns:py="http://genshi.edgewall.org/"
     16      xmlns:i18n="http://genshi.edgewall.org/i18n"
    1617      xmlns:xi="http://www.w3.org/2001/XInclude">
    1718  <xi:include href="prefs.html" />
    1819  <head>
     
    3233        <tr>
    3334          <th></th>
    3435          <td>
    35             <p class="hint">The session key is used to identify stored custom
    36             settings and session data on the server. Although it is
    37             automatically generated by default, you may change it to something
    38             easier to remember at any time if you wish to load your settings
    39             in a different web browser.</p>
     36            <p class="hint" i18n:msg="">The session key is used to <!--!
     37            -->identify stored custom settings and session data on <!--!
     38            -->the server. Although it is automatically generated <!--!
     39            -->by default, you may change it to something easier <!--!
     40            -->to remember at any time if you wish to load your <!--!
     41            -->settings in a different web browser.</p>
    4042          </td>
    4143        </tr>
    4244        <tr class="field">
     
    4951        <tr>
    5052          <th></th>
    5153          <td>
    52             <p class="hint">You may load a previously created session by enteri
    53             corresponding session key below. This lets you share settings betwe
    54             multiple computers and web browsers.</p>
     54            <p class="hint" i18n:msg="">You may load a previously <!--!
     55            -->created session by entering the corresponding session <!--!
     56            -->key below. This lets you share settings between <!--!
     57            -->multiple computers and web browsers.</p>
    5558          </td>
    5659        </tr>
    5760      </table>

However, the above patch prevents line breaks in the extracted message whereas just removing the indentation does not:

  • trac/prefs/templates/prefs_advanced.html

    diff --git a/trac/prefs/templates/prefs_advanced.html b/trac/prefs/templates/pre
    index 82fb2af..dc5ffae 100644
    a b  
    3232        <tr>
    3333          <th></th>
    3434          <td>
    35             <p class="hint">The session key is used to identify stored custom
    36             settings and session data on the server. Although it is
    37             automatically generated by default, you may change it to something
    38             easier to remember at any time if you wish to load your settings
    39             in a different web browser.</p>
     35            <p class="hint">
     36The session key is used to identify stored custom settings and
     37session data on the server. Although it is automatically generated
     38by default, you may change it to something easier to remember at
     39any time if you wish to load your settings in a different web
     40browser.</p>
    4041          </td>
    4142        </tr>
    4243        <tr class="field">
     
    4950        <tr>
    5051          <th></th>
    5152          <td>
    52             <p class="hint">You may load a previously created session by enteri
    53             corresponding session key below. This lets you share settings betwe
    54             multiple computers and web browsers.</p>
     53            <p class="hint">
     54You may load a previously created session by entering the
     55corresponding session key below. This lets you share settings
     56between multiple computers and web browsers.</p>
    5557          </td>
    5658        </tr>
    5759      </table>

Does anyone have a preference?

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:6 in reply to:  3 Changed 4 years ago by Ryan J Ollos

Replying to psuter:

… Requested in #4263, #956, #1752.)

The issue with those two TracLinks is reported in #11816.

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:7 Changed 4 years ago by Ryan J Ollos

I'm seeing some strange behavior with the Git repository for the past few weeks. The log shows commits from the 0.12-stable branch:

$ git log --oneline mirror/trunk
3f8b606 1.1.3dev: merged [13269] from 1.0-stable (fix for `WikiPageTestCase.test
2ca7f2f 0.12.7dev: fixed rare failure in `WikiPageTestCase.test_update_page`
c673034 1.1.3dev: merged [13266] from 1.0-stable (fix for #11797)
1f21f4a 1.1.3dev: record-only merge on [13264]
f4d466d 1.1.3dev: Merged [13262] from 1.0-stable. Refs #11813.
c563250 1.1.3dev: Merged [13260] from 1.0-stable. Refs #11612.
0dd3041 1.1.3dev: merged r13251 from 1.0-stable (`make status` improvements)
fc35fb8 0.12.7dev: use a script to display packages versions in `make status`
8b8f5c2 0.12.7dev: improve `make status` output

A simple interactive rebase with all of the revisions left as pick fails:

$ git rebase -i HEAD~5
error: could not apply 2ca7f2f... 0.12.7dev: fixed rare failure in `WikiPageTestCase.test_update_page`

When you have resolved this problem, run "git rebase --continue".
If you prefer to skip this patch, run "git rebase --skip" instead.
To check out the original branch and stop rebasing, run "git rebase --abort".
Could not apply 2ca7f2f2bbb708bc6ead53b0c4218577dbc7ca82... 0.12.7dev: fixed rare failure in `WikiPageTestCase.test_update_page`

comment:8 Changed 4 years ago by Ryan J Ollos

Release Notes: modified (diff)
Status: newassigned

Removed some unnecessary tbody tags in [13271:13272]. Improved alignment of elements on the Date & Time and Language preference panels in [13273:13276].

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:9 Changed 4 years ago by Ryan J Ollos

Those automated builds are paying off. I didn't have LXML installed locally when testing [13273] before commit.

Changed 4 years ago by Ryan J Ollos

Attachment: DateSelectorsOnSameLine.png added

comment:10 Changed 4 years ago by Ryan J Ollos

Some changes for the trunk in log:rjollos.git:t11813.3:

  • Added Localization preferences panel.
  • Implemented #11659 (functional test needed and could be backported to 1.0-stable).
  • Moved Default handler to the User Interface preferences panel.

I renamed Date relative/absolute format to Time format with the aim of shortening the label length. That isn't a great name, but the only other ideas I have are:

  • Date presentation
  • Put both date_format and dateinfo_format selectors on the same line with single label. This would work better if we put the actual default value next to Default, like Default handler does: Default (WikiModule). However, one of the selectors wouldn't have a label, so that probably isn't good for accessibility.

Regarding the placement of the new selector from #11659, technically Default date relative/absolute format might not be considered in the category of Localization, but that's a technical detail, so I'm interested to hear whether others think it is easy to find. I'd be okay with putting it on a Localization panel if it can be easily located.

comment:11 Changed 4 years ago by Jun Omae

I get the following failure on [13275]. It seems that this occurs if compiled catalogs exist. Autobuilds have no error because .travis.yml doesn't have step to compile catalogs, e.g. make compile.

======================================================================
FAIL: Test for regression of http://trac.edgewall.org/ticket/11337
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/run/shm/d130dddc5c571492f945eeab2c37f86921e071c4/py25-sqlite/trac/prefs/tests/functional.py", line 82, in runTest
    tc.find(language_select)
  File "/run/shm/d130dddc5c571492f945eeab2c37f86921e071c4/py25-sqlite/trac/tests/functional/better_twill.py", line 227, in better_find
    (unicode(e), filename))
TwillAssertionError: no match to '<select name="language">' at file:///run/shm/d130dddc5c571492f945eeab2c37f86921e071c4/py25-sqlite/testenv/trac/log/RegressionTestTicket11337.html
Last edited 4 years ago by Jun Omae (previous) (diff)

comment:12 Changed 4 years ago by Ryan J Ollos

Thanks for the report. Fixed in [13277:13278]

comment:13 in reply to:  7 Changed 4 years ago by Christian Boos

Replying to rjollos:

I'm seeing some strange behavior with the Git repository for the past few weeks. The log shows commits from the 0.12-stable branch:

$ git log --oneline mirror/trunk
3f8b606 1.1.3dev: merged [13269] from 1.0-stable (fix for `WikiPageTestCase.test
2ca7f2f 0.12.7dev: fixed rare failure in `WikiPageTestCase.test_update_page`
c673034 1.1.3dev: merged [13266] from 1.0-stable (fix for #11797)
1f21f4a 1.1.3dev: record-only merge on [13264]
f4d466d 1.1.3dev: Merged [13262] from 1.0-stable. Refs #11813.
c563250 1.1.3dev: Merged [13260] from 1.0-stable. Refs #11612.
0dd3041 1.1.3dev: merged r13251 from 1.0-stable (`make status` improvements)
fc35fb8 0.12.7dev: use a script to display packages versions in `make status`
8b8f5c2 0.12.7dev: improve `make status` output

It's expected, git-svn does some merging on its own.

Try:

$ git log --graph --oneline --decorate 3f8b606 
*   3f8b606 1.1.3dev: merged [13269] from 1.0-stable (fix for `WikiPageTestCase.test_update_page`)
|\
| * 2ca7f2f (origin/0.12-stable) 0.12.7dev: fixed rare failure in `WikiPageTestCase.test_update_page`
* | c673034 1.1.3dev: merged [13266] from 1.0-stable (fix for #11797)
* | 1f21f4a 1.1.3dev: record-only merge on [13264]
* | f4d466d 1.1.3dev: Merged [13262] from 1.0-stable. Refs #11813.
* | c563250 1.1.3dev: Merged [13260] from 1.0-stable. Refs #11612.
* |   0dd3041 1.1.3dev: merged r13251 from 1.0-stable (`make status` improvements)
|\ \
| |/
| * fc35fb8 0.12.7dev: use a script to display packages versions in `make status`
| * 8b8f5c2 0.12.7dev: improve `make status` output
* | 121aaf2 1.1.3dev: Fixed logic that was inverted in [13220]. Refs #11794.
* | a1143e4 1.1.3dev: merged [13243] from 1.0-stable (fix for #11805)

or look at log:cboos.git@3705f00, to compare with log:rjollos.git@t11813.2.

Rebasing across merges is acrobatic and most certainly not what you wanted to do ;-)

Last edited 4 years ago by Christian Boos (previous) (diff)

comment:14 in reply to:  11 ; Changed 4 years ago by Jun Omae

Autobuilds have no error because .travis.yml doesn't have step to compile catalogs, e.g. make compile.

Added make compile to .travis.yml in [13279-13281].

comment:15 Changed 4 years ago by Ryan J Ollos

Removed whitespace from extracted messages in [13290], merged in [13291]. See comment:5 for details.

comment:16 Changed 4 years ago by Ryan J Ollos

New extraction on 1.0-stable in [13293], on trunk in [13295].

comment:17 Changed 4 years ago by Ryan J Ollos

I missed fixing one indentation change in [13290]: trac/wiki/templates/wiki_view.html:46. It will be fixed in #11825.

Changed 4 years ago by Ryan J Ollos

Attachment: NewBasicSettings.png added

Changed 4 years ago by Ryan J Ollos

Attachment: NewPrefsLocalization.png added

Changed 4 years ago by Ryan J Ollos

Attachment: NewPrefsUserInterface.png added

comment:18 Changed 4 years ago by Ryan J Ollos

Fixed some minor issues and rebased changes in log:rjollos.git:t11813.4. Here are screen captures of the proposed changes.

comment:19 in reply to:  14 ; Changed 4 years ago by Ryan J Ollos

Replying to jomae:

Autobuilds have no error because .travis.yml doesn't have step to compile catalogs, e.g. make compile.

Added make compile to .travis.yml in [13279-13281].

There are some intermittently failing builds.

It looks like the order of catalog compilation is random, or at least unpredictable. Here are two examples from the TravisCI builds:

python setup.py  compile_catalog  compile_catalog_js  compile_catalog_tracini generate_messages_js
running compile_catalog
compiling catalog 'trac/locale/ro/LC_MESSAGES/messages.po' to 'trac/locale/ro/LC_MESSAGES/messages.mo'
compiling catalog 'trac/locale/hu/LC_MESSAGES/messages.po' to 'trac/locale/hu/LC_MESSAGES/messages.mo'
compiling catalog 'trac/locale/es_MX/LC_MESSAGES/messages.po' to 'trac/locale/es_MX/LC_MESSAGES/messages.mo'
...
python setup.py  compile_catalog  compile_catalog_js  compile_catalog_tracini generate_messages_js
running compile_catalog
compiling catalog 'trac/locale/ko/LC_MESSAGES/messages.po' to 'trac/locale/ko/LC_MESSAGES/messages.mo'
compiling catalog 'trac/locale/el/LC_MESSAGES/messages.po' to 'trac/locale/el/LC_MESSAGES/messages.mo'
compiling catalog 'trac/locale/ru/LC_MESSAGES/messages.po' to 'trac/locale/ru/LC_MESSAGES/messages.mo'
...

The order is repeatable when run locally. The order of languages returned by get_available_locales corresponds to the compilation order:

$ make compilepython setup.py  compile_catalog  compile_catalog_js  compile_catalog_tracini generate_messages_js
running compile_catalog
compiling catalog 'trac/locale/nl/LC_MESSAGES/messages.po' to 'trac/locale/nl/LC_MESSAGES/messages.mo'
compiling catalog 'trac/locale/fi/LC_MESSAGES/messages.po' to 'trac/locale/fi/LC_MESSAGES/messages.mo'
compiling catalog 'trac/locale/ca/LC_MESSAGES/messages.po' to 'trac/locale/ca/LC_MESSAGES/messages.mo'
compiling catalog 'trac/locale/hu/LC_MESSAGES/messages.po' to 'trac/locale/hu/LC_MESSAGES/messages.mo'
...
>>> from trac.util.translation import get_available_locales
>>> get_available_locales()
['nl', 'fi', 'ca', 'hu', 'en_GB', 'nb', 'it', 'zh_CN', 'pt_BR', 'ko', 'hy', 'ro', 'da', 'sv', 'ru', 'es_AR', 'he', 'pl', 'es_MX', 'de', 'sl', 'ja', 'et', 'el', 'en_US', 'gl', 'uk', 'eo', 'pt', 'zh_TW', 'tr', 'cs', 'fr', 'es']

The ro language doesn't have a translation for Your preferences have been saved. In cases where the build fails, it looks like the ro locale has been compiled first. The translation selected in RegressionTestTicket11515 is therefore ro: tags/trac-1.0.2/trac/prefs/tests/functional.py@:116-117#L107, and the untranslated message Your preferences have been saved is seen.

There are a few other languages that don't have a translation for that line.

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:20 in reply to:  19 Changed 4 years ago by Jun Omae

Replying to rjollos:

There are a few other languages that don't have a translation for that line.

Thanks for the investigation. That is introduced in [12578] (#11515) by me. I'll fix it.

comment:21 Changed 4 years ago by Ryan J Ollos

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed to trunk in [13323:13326].

comment:22 Changed 4 years ago by Peter Suter

Since [13323] combined the Date & Time and Language preference panels as Localization, RegressionTestTicket11337 (#11337) fails if Babel is not installed:

$ python trac\prefs\tests\functional.py
....F..
======================================================================
FAIL: Test for regression of http://trac.edgewall.org/ticket/11337
----------------------------------------------------------------------
Traceback (most recent call last):
  File "trac\prefs\tests\functional.py", line 154, in runTest
    tc.notfind(language_tab)
  File "trac\tests\functional\better_twill.py", line 236, in better_notfind
    (unicode(e), filename))
TwillAssertionError: match to '<li id="tab_localization">' at testenv/trac/log/RegressionTestTicket11337.html

----------------------------------------------------------------------
Ran 7 tests in 9.502s

FAILED (failures=1)

The merged Localization tab is always shown.

  • trac/prefs/tests/functional.py

    diff -r 4a0829499bba trac/prefs/tests/functional.py
    a b  
    115115class RegressionTestTicket11337(FunctionalTwillTestCaseSetup):
    116116    def runTest(self):
    117117        """Test for regression of http://trac.edgewall.org/ticket/11337
    118         The preferences panel will only be visible when Babel is installed
     118        The preferences panel will be different when Babel is installed
    119119        or for a user that has `TRAC_ADMIN`.
    120120        """
    121121        from trac.util.translation import has_babel, get_available_locales
     
    141141            tc.find(disabled_language_select)
    142142            tc.notfind(catalog_hint)
    143143
    144         # For users without TRAC_ADMIN, the Language tab should only be
    145         # present when Babel is installed
     144        # For users without TRAC_ADMIN
    146145        self._tester.go_to_preferences()
    147         language_tab = '<li id="tab_localization">'
     146        localization_tab = '<li id="tab_localization">'
    148147        try:
    149148            self._tester.logout()
     149            tc.find(localization_tab)
    150150            if has_babel:
    151                 tc.find(language_tab)
    152151                tc.notfind(catalog_hint)
    153             else:
    154                 tc.notfind(language_tab)
    155152        finally:
    156153            self._tester.login('admin')
    157154

I'm not sure if this test still makes sense at all.

comment:23 in reply to:  22 Changed 4 years ago by Ryan J Ollos

Replying to psuter:

I'm not sure if this test still makes sense at all.

You are right, the test needed to be reworked. Fixed in [13440]. Thanks.

Side note: [13323] incorrectly references ticket #10674 rather than this ticket.

comment:24 Changed 4 years ago by Ryan J Ollos

Regression reported in #11921.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.