Ticket #7811 (closed defect: fixed)
Worksheet list CSS: Account for special characters in login names
| Reported by: | mpatel | Owned by: | was |
|---|---|---|---|
| Priority: | major | Milestone: | sage-4.3.1 |
| Component: | notebook | Keywords: | |
| Cc: | robert.marik, timdumol, was | Work issues: | |
| Report Upstream: | N/A | Reviewers: | Robert Marik |
| Authors: | Mitesh Patel | Merged in: | sagenb-0.4.8 |
| Dependencies: | Stopgaps: |
Description
We need to account for this difference
$ grep compile twist.py template.py
twist.py:re_valid_username = re.compile('[a-z|A-Z|0-9|_|.|@]*')
template.py:css_illegal_re = re.compile(r'[^-A-Za-z_0-9]')
when processing the checkboxes in a worksheet listing. Otherwise, the Archive, Stop, and Delete buttons will not work for users whose login names contain dots (.) or at signs (@).
This is a follow-up to #7332. See sage-devel for the bug report.
Attachments
-
trac_7811-escape_ws_list_ids.patch
(1.2 KB) -
added by mpatel 3 years ago.
- Escape /, @, and . in worksheet list CSS IDs. sagenb repo.
-
trac_7811-escape_ws_list_ids_v2.patch
(1.3 KB) -
added by mpatel 3 years ago.
- More general RegExp?. Replaces previous.
Change History
Changed 3 years ago by mpatel
-
attachment
trac_7811-escape_ws_list_ids.patch
added
comment:1 Changed 3 years ago by mpatel
- Cc robert.marik, timdumol, was added
- Status changed from new to needs_review
- Authors set to Mitesh Patel
Please let me know if I've missed any other characters.
comment:2 Changed 3 years ago by timdumol
Why not use /[-A-Za-z_0-9]/g ? If the regexp for usernames is updated for all valid emails, then '+' will be allowed for usernames, which is illegal as a CSS id.
Changed 3 years ago by mpatel
-
attachment
trac_7811-escape_ws_list_ids_v2.patch
added
More general RegExp?. Replaces previous.
comment:4 Changed 3 years ago by robert.marik
Works for me. Is it neceassary to run ./sage -t even when upgrading spkg package?
comment:5 follow-up: ↓ 7 Changed 3 years ago by mpatel
If you mean sage -b --- it's necessary only for changes to the main Sage library, under SAGE_ROOT/devel/sage, but not if you're just installing a spkg.
comment:6 Changed 3 years ago by timdumol
Ideally this should be tested with sage -t -sagenb when #7650 comes in, or with a Selenium test (sagenb.testing), specifically in sagenb.testing.tests.test_accounts.
comment:7 in reply to: ↑ 5 Changed 3 years ago by robert.marik
Replying to mpatel:
If you mean sage -b --- it's necessary only for changes to the main Sage library, under SAGE_ROOT/devel/sage, but not if you're just installing a spkg.
No, I meant actually sage -t. I wondered, if I can give positive review without doctesting.
comment:8 Changed 3 years ago by robert.marik
- Status changed from needs_review to positive_review
- Reviewers set to Robert Marik
Wors fine. Tests passed. Doctests are not meaningful for this patch. Positive review.
Escape /, @, and . in worksheet list CSS IDs. sagenb repo.