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

Worksheet list CSS: Account for special characters in login names #7811

Closed
qed777 mannequin opened this issue Jan 1, 2010 · 14 comments
Closed

Worksheet list CSS: Account for special characters in login names #7811

qed777 mannequin opened this issue Jan 1, 2010 · 14 comments

Comments

@qed777
Copy link
Mannequin

qed777 mannequin commented Jan 1, 2010

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.

CC: @robert-marik @TimDumol @williamstein

Component: notebook

Author: Mitesh Patel

Reviewer: Robert Marik

Merged: sagenb-0.4.8

Issue created by migration from https://trac.sagemath.org/ticket/7811

@qed777 qed777 mannequin added this to the sage-4.3.1 milestone Jan 1, 2010
@qed777 qed777 mannequin added c: user interface labels Jan 1, 2010
@qed777 qed777 mannequin assigned williamstein Jan 1, 2010
@qed777
Copy link
Mannequin Author

qed777 mannequin commented Jan 1, 2010

Escape /, @, and . in worksheet list CSS IDs. sagenb repo.

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Jan 1, 2010

Author: Mitesh Patel

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Jan 1, 2010

comment:1

Attachment: trac_7811-escape_ws_list_ids.patch.gz

Please let me know if I've missed any other characters.

@qed777 qed777 mannequin added the s: needs review label Jan 1, 2010
@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Jan 2, 2010

comment:2

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.

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Jan 2, 2010

comment:3

Excellent point. I'll update the patch.

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Jan 2, 2010

Attachment: trac_7811-escape_ws_list_ids_v2.patch.gz

More general RegExp. Replaces previous.

@robert-marik
Copy link
Mannequin

robert-marik mannequin commented Jan 2, 2010

comment:4

Works for me. Is it neceassary to run ./sage -t even when upgrading spkg package?

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Jan 2, 2010

comment:5

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.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Jan 2, 2010

comment:6

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.

@robert-marik
Copy link
Mannequin

robert-marik mannequin commented Jan 2, 2010

comment:7

Replying to @qed777:

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.

@robert-marik
Copy link
Mannequin

robert-marik mannequin commented Jan 3, 2010

Reviewer: Robert Marik

@robert-marik
Copy link
Mannequin

robert-marik mannequin commented Jan 3, 2010

comment:8

Wors fine. Tests passed. Doctests are not meaningful for this patch. Positive review.

@williamstein
Copy link
Contributor

comment:9

merged into sagenb-0.4.8

@sagetrac-mvngu
Copy link
Mannequin

sagetrac-mvngu mannequin commented Jan 7, 2010

Merged: sagenb-0.4.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant