Implement <input type="url">

RESOLVED FIXED in mozilla2.0b5

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
11 years ago
7 years ago

People

(Reporter: WeirdAl, Assigned: mounir)

Tracking

(Blocks: 4 bugs, {dev-doc-complete, html5})

Trunk
mozilla2.0b5
dev-doc-complete, html5
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [parity-opera], URL)

Attachments

(1 attachment, 7 obsolete attachments)

(Reporter)

Description

11 years ago
I'm willing to take a shot at implementing <input type="uri"> as described in Web Forms 2.0.
(Reporter)

Updated

11 years ago
Summary: Implement <input type="uri"> → Implement <input type="url">
(Reporter)

Comment 1

11 years ago
So I'm having a little trouble with overriding HTML layout.

http://mozilla.pastebin.ca/93708 XML binding (inputBinding.xml)
http://mozilla.pastebin.ca/93709 XHTML test page (inputBinding.xhtml)

The binding works for <implementation/> (I can execute a dump() safely via the constructor), but the <content/> of the binding never appears.

I did some preliminary debugging, and discovered nsCSSFrameConstructor::CreateInputFrame.  control->GetType() returns NS_FORM_INPUT_TEXT, and so CreateInputFrame proceeds to construct the HTML text input frame instead of the XBL-based layout I want.

roc, I think this is something you can help with.  I'm thinking this really is a separate bug, but I want someone who's familiar with layout to affirm that before I file.
Yeah, nsCSSFrameConstructor needs to be changed. Ping me about it in a couple of weeks, OK?
Actually, maybe it doesn't. Try changing the XBL base tag of your binding to something other than an HTML <input>, e.g., a <span>
(Reporter)

Comment 4

11 years ago
If you mean change:

    <p>HTML input: 
      <input type="uri" size="20" value="http://www.w3.org"/>
    </p>

to

    <p>HTML input: 
      <span type="uri" size="20" value="http://www.w3.org"/>
    </p>

then yes, it does work, but it defeats the purpose of Web Forms 2.0 - which is about extending the HTML input types.

If you mean changing the binding from:

    <content>
      <html:input type="text" xbl:inherits="value,size"/>
      <xul:hbox><xul:button label="Test"/></xul:hbox>
    </content>

to:

    <content>
      <html:span>
        <html:input type="text" xbl:inherits="value,size"/>
        <xul:hbox><xul:button label="Test" oncommand="alert('button clicked')"/></xul:hbox>
      </html:span>
    </content>

That doesn't affect the layout of the HTML input widget at this point.
I think I mean the latter.

Updated

9 years ago
Component: DOM: HTML → DOM: Core & HTML
QA Contact: ian → general

Comment 6

8 years ago
Gmail is now using this, and Opera supports it: http://my.opera.com/dbloom/blog/2009/09/02/secret-opera-only-feature-in-gmail

Updated

8 years ago
Assignee: ajvincent → nobody

Comment 7

7 years ago
The way Opera has done it, with the autocomplete URL feature is very good. Do you think we could get the Awesomebar functionality in there? (And perhaps an icon, like Opera have too.)
(Assignee)

Updated

7 years ago
Blocks: 558370
(Assignee)

Updated

7 years ago
Blocks: 558651
(Assignee)

Updated

7 years ago
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Depends on: 345624
OS: Windows XP → All
Hardware: x86 → All
(Assignee)

Updated

7 years ago
Keywords: html5
Whiteboard: [parity-opera]
(Assignee)

Updated

7 years ago
Blocks: 559309
(Assignee)

Comment 8

7 years ago
According to HTML5 specs, a valid URL is a valid URI (RFC3986) or a valid IRI (RFC3987). I'm wondering if url parser are following these RFC's.
I've seen only one comment related to one of these RFC's in the code base: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#2183
I believe we implement more or less RFC 1738 last I checked.

Updated

7 years ago
Blocks: 559745
(Assignee)

Comment 10

7 years ago
Created attachment 439737 [details] [diff] [review]
Tests v1
Attachment #439737 - Flags: review?(jonas)
(Assignee)

Comment 11

7 years ago
Created attachment 439738 [details] [diff] [review]
Patch v1

This patch is unfortunately not checking the URI as the HTML5 specifications want it to. Considering the needed RFC's (3886 and 3987) are improving the RFC's we are following at the moment (see Boris' comment) it is probably better to check the URI as we can and improve that later.
Attachment #439738 - Flags: review?(jonas)
(Assignee)

Comment 12

7 years ago
Created attachment 440191 [details] [diff] [review]
Tests v1.1

The tests are now adding url-related tests for the required attribute tests.
Attachment #439737 - Attachment is obsolete: true
Attachment #440191 - Flags: review?(jonas)
Attachment #439737 - Flags: review?(jonas)
(Assignee)

Comment 13

7 years ago
Created attachment 440192 [details] [diff] [review]
Patch v1.1

This patch has to be applied on top of the required attribute patch (bug 34582).
Attachment #439738 - Attachment is obsolete: true
Attachment #440192 - Flags: review?(jonas)
Attachment #439738 - Flags: review?(jonas)
(Assignee)

Updated

7 years ago
Depends on: 34582
(Assignee)

Updated

7 years ago
Depends on: 345822
No longer depends on: 34582
Comment on attachment 440192 [details] [diff] [review]
Patch v1.1

>+    /**
>+     * TODO:
>+     * The URL is not checked as the HTML5 specifications want it to be because
>+     * there is no code to check for a valid URI/IRI according to 3986 and 3987
>+     * RFC's at the moment.
>+     *
>+     * RFC 3987 (IRI) implementation: bug 42899
>+     *
>+     * HTML5 specifications:
>+     * http://dev.w3.org/html5/spec/infrastructure.html#valid-url
>+     */

Could you file a bug and reference it here?


>@@ -3498,19 +3534,26 @@ nsHTMLInputElement::GetValidationMessage
>       rv = nsContentUtils::GetLocalizedString(nsContentUtils::eDOM_PROPERTIES,
>                                               key.get(), message);
>       aValidationMessage = message;
>       break;
>     }
>     case VALIDATION_MESSAGE_TYPE_MISMATCH:
>     {
>       nsXPIDLString message;
>+      nsCAutoString key;
>+      if (mType == NS_FORM_INPUT_EMAIL) {
>+        key.Assign("ElementSuffersFromInvalidEmail");
>+      } else if (mType == NS_FORM_INPUT_URL) {
>+        key.Assign("ElementSuffersFromInvalidURL");
>+      } else {
>+        return NS_ERROR_UNEXPECTED;
>+      }

Use key.AssignLiteral(...)
Comment on attachment 440192 [details] [diff] [review]
Patch v1.1

With that, looks good.

Though please file a followup to create actual UI for this. We probably want to hook up the global history as validation.
Attachment #440192 - Flags: review?(jonas) → review+
Hmm.. though.. doesn't the spec require that the user can only enter valid URIs? In other words, if the user types something that isn't a valid uri then .value shouldn't be updated until this is corrected?
(Assignee)

Comment 17

7 years ago
(In reply to comment #16)
> Hmm.. though.. doesn't the spec require that the user can only enter valid
> URIs? In other words, if the user types something that isn't a valid uri then
> .value shouldn't be updated until this is corrected?

No, email and url types can suffer from type mismatch so the user can type anything. If this is not valid, the element will suffer from type mismatch. Input types which can't have a valid value are the ones with a specific UI which are number, range, color and all date/time ones.

And thank you for the review :)
Attachment #440191 - Flags: review?(jonas) → review+
Comment on attachment 440191 [details] [diff] [review]
Tests v1.1

Tests look good. Though we'll need tests for UI once that arrives.
Ah, you are indeed correct.

It does say though that the UA may "escape characters entered by the user so that the value is always a valid absolute URL" which I think would be cool to do. Though definitely not required in this bug.
(Assignee)

Updated

7 years ago
Blocks: 561586
(Assignee)

Updated

7 years ago
Blocks: 561620
(Assignee)

Comment 20

7 years ago
Created attachment 441348 [details] [diff] [review]
Tests v1.2

Minor changes in the tests: I've added a check for the invalid event.
(Assignee)

Updated

7 years ago
Attachment #440191 - Attachment is obsolete: true
(Assignee)

Comment 21

7 years ago
Created attachment 441349 [details] [diff] [review]
Patch v1.2

r=sicking

Requested changes have been done.
Attachment #440192 - Attachment is obsolete: true
Attachment #441349 - Flags: superreview?(Olli.Pettay)

Updated

7 years ago
Attachment #441349 - Flags: superreview?(Olli.Pettay) → superreview+
(Assignee)

Updated

7 years ago
Blocks: 562625

Updated

7 years ago
Blocks: 566348
Keywords: dev-doc-complete
Keywords: dev-doc-complete → dev-doc-needed
(Assignee)

Updated

7 years ago
Blocks: 558788
(Assignee)

Updated

7 years ago
Blocks: 561635
(Assignee)

Comment 22

7 years ago
Created attachment 460867 [details] [diff] [review]
Patch v1.3

Updated to apply against current tip.
Merged with test patch.
Attachment #441348 - Attachment is obsolete: true
Attachment #441349 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Blocks: 583610
(Assignee)

Comment 23

7 years ago
Created attachment 461908 [details] [diff] [review]
Patch v1.3

In the last update, I've added value sanitizing related code and tests. This is now in bug 583610 to simplify the review process.
Attachment #460867 - Attachment is obsolete: true
(Assignee)

Comment 24

7 years ago
This is now in the tree.
rev: http://hg.mozilla.org/mozilla-central/rev/acc8ccecdc90
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b5
Keywords: dev-doc-needed → dev-doc-complete

Updated

7 years ago
Blocks: 581596
You need to log in before you can comment on or make changes to this bug.