Skip to content

Commit

Permalink
Fix oppia#2584: Replace default text while inserting links with place…
Browse files Browse the repository at this point in the history
…holder (oppia#6286)

* Fixes oppia#2584: Replace default text while inserting links with placeholder

* Making minor changes

* Modifying backend test

* Fixing e2e test errors

* Making changes

* Creating new html file for sanitized url

* Adding validator to SanitizeUrl object

* Adding spaces

* Adding tests

* Adding else statement

* Replacing list_editor with sanitized_url_editor
  • Loading branch information
abhaygupta97 authored and seanlip committed Mar 3, 2019
1 parent 2812471 commit 9ed174b
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 45 deletions.
2 changes: 1 addition & 1 deletion assets/rich_text_components_definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ var richTextComponents = {
"type": "custom",
"obj_type": "SanitizedUrl"
},
"default_value": "https://www.example.com"
"default_value": ""
}, {
"name": "text",
"description": "The link text. If left blank, the link URL will be used.",
Expand Down
16 changes: 11 additions & 5 deletions core/domain/rte_component_registry_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,17 @@ def _validate_customization_arg_specs(self, customization_arg_specs):
apply_custom_validators=False))

if ca_spec['schema']['type'] == 'custom':
obj_class = obj_services.Registry.get_object_class_by_type(
ca_spec['schema']['obj_type'])
self.assertEqual(
ca_spec['default_value'],
obj_class.normalize(ca_spec['default_value']))
# Default value of SanitizedUrl obj_type may be empty. The empty
# string is not considered valid for this object, so we don't
# attempt to normalize it.
if ca_spec['schema']['obj_type'] == 'SanitizedUrl':
self.assertEqual(ca_spec['default_value'], '')
else:
obj_class = obj_services.Registry.get_object_class_by_type(
ca_spec['schema']['obj_type'])
self.assertEqual(
ca_spec['default_value'],
obj_class.normalize(ca_spec['default_value']))

def _listdir_omit_ignored(self, directory):
"""List all files and directories within 'directory', omitting the ones
Expand Down
6 changes: 6 additions & 0 deletions extensions/objects/models/objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,12 @@ class SanitizedUrl(BaseObject):

SCHEMA = {
'type': 'unicode',
'validators': [{
'id': 'is_nonempty'
}],
'ui_config': {
'placeholder': 'https://www.example.com'
},
'post_normalizers': [{
'id': 'sanitize_url'
}]
Expand Down
41 changes: 9 additions & 32 deletions extensions/objects/templates/SanitizedUrlEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,50 +12,27 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// This directive is based on the unicodeStringEditor one.

oppia.directive('sanitizedUrlEditor', [
'UrlInterpolationService', 'OBJECT_EDITOR_URL_PREFIX',
function(UrlInterpolationService, OBJECT_EDITOR_URL_PREFIX) {
// Editable URL directive.
return {
restrict: 'E',
scope: {
getInitArgs: '&',
value: '='
},
templateUrl: UrlInterpolationService.getExtensionResourceUrl(
'/objects/templates/unicode_string_editor_directive.html'),
'/objects/templates/sanitized_url_editor_directive.html'),
controller: ['$scope', function($scope) {
$scope.initArgs = $scope.getInitArgs();
$scope.$watch('initArgs', function(newValue) {
$scope.largeInput = false;
if (newValue && newValue.largeInput) {
$scope.largeInput = newValue.largeInput;
}
});

$scope.$watch('value', function(newValue) {
$scope.localValue = {
label: String(newValue) || ''
};
}, true);

$scope.alwaysEditable = true;

$scope.$watch('localValue.label', function(newValue) {
$scope.value = newValue;
});

$scope.$on('externalSave', function() {
var currentValue = String($scope.localValue.label);
if ($scope.active) {
$scope.replaceValue(currentValue);
// The $scope.$apply() call is needed to propagate the replaced
// value.
$scope.$apply();
$scope.SCHEMA = {
type: 'unicode',
validators: [{
id: 'is_nonempty'
}],
ui_config: {
placeholder: 'https://www.example.com'
}
});
};
}]
};
}]);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<schema-based-unicode-editor local-value="value" validators="SCHEMA.validators"
ui-config="SCHEMA.ui_config">
</schema-based-unicode-editor>
25 changes: 18 additions & 7 deletions schema_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ def normalize_against_schema(obj, schema, apply_custom_validators=True):
from core.domain import obj_services # pylint: disable=relative-import
obj_class = obj_services.Registry.get_object_class_by_type(
schema[SCHEMA_KEY_OBJ_TYPE])
normalized_obj = obj_class.normalize(obj)
if not apply_custom_validators:
normalized_obj = normalize_against_schema(
obj, obj_class.SCHEMA, apply_custom_validators=False)
else:
normalized_obj = obj_class.normalize(obj)
elif schema[SCHEMA_KEY_TYPE] == SCHEMA_TYPE_DICT:
assert isinstance(obj, dict), ('Expected dict, received %s' % obj)
expected_dict_keys = [
Expand Down Expand Up @@ -199,11 +203,11 @@ def normalize_spaces(obj):
"""Collapses multiple spaces into single spaces.
Args:
obj: a string.
obj: a string.
Returns:
a string that is the same as `obj`, except that each block of
whitespace is collapsed into a single space character.
a string that is the same as `obj`, except that each block of
whitespace is collapsed into a single space character.
"""
return ' '.join(obj.split())

Expand All @@ -212,12 +216,19 @@ def sanitize_url(obj):
"""Takes a string representing a URL and sanitizes it.
Args:
obj: a string representing a URL.
obj: a string representing a URL.
Returns:
An empty string if the URL does not start with http:// or https://.
Otherwise, returns the original URL.
An empty string if the URL does not start with http:// or https://
except when the string is empty. Otherwise, returns the original
URL.
Raises:
AssertionError: The string is non-empty and does not start with
http:// or https://
"""
if obj == '':
return obj
url_components = urlparse.urlsplit(obj)
quoted_url_components = (
urllib.quote(component) for component in url_components)
Expand Down
33 changes: 33 additions & 0 deletions schema_utils_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -581,3 +581,36 @@ def test_notification_email_list_validator(self):
invalid_vals = [[u'admin@oppia'], big_email_list,
[u'[email protected]'], [u'[email protected]']]
self.check_normalization(schema, mappings, invalid_vals)

def test_sanitize_url(self):
"""Tests that sanitize_url method correctly sanitizes a string
representing a URL and raises error for invalid strings.
"""
sanitize_url = schema_utils.Normalizers.get('sanitize_url')
self.assertEqual(
sanitize_url('https://www.oppia.org/splash/'),
'https://www.oppia.org/splash/')

self.assertEqual(
sanitize_url('http://www.oppia.org/splash/'),
'http://www.oppia.org/splash/')

self.assertEqual(
sanitize_url('http://example.com/~path;parameters?q=arg#fragment'),
'http://example.com/%7Epath%3Bparameters?q%3Darg#fragment')

self.assertEqual(sanitize_url(''), '')

# Raise AssertionError if string does not start with http:// or
# https://.
with self.assertRaisesRegexp(
AssertionError,
'Invalid URL: Sanitized URL should start with \'http://\' or'
' \'https://\'; received oppia.org'):
sanitize_url('oppia.org')

with self.assertRaisesRegexp(
AssertionError,
'Invalid URL: Sanitized URL should start with \'http://\' or'
' \'https://\'; received www.oppia.org'):
sanitize_url('www.oppia.org')

0 comments on commit 9ed174b

Please sign in to comment.