Closed Bug 344615 Opened 14 years ago Closed 10 years ago

Implement <input type="url">

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b5

People

(Reporter: WeirdAl, Assigned: mounir)

References

(Blocks 3 open bugs, )

Details

(Keywords: dev-doc-complete, html5, Whiteboard: [parity-opera])

Attachments

(1 file, 7 obsolete files)

I'm willing to take a shot at implementing <input type="uri"> as described in Web Forms 2.0.
Summary: Implement <input type="uri"> → Implement <input type="url">
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>
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.
Component: DOM: HTML → DOM: Core & HTML
QA Contact: ian → general
Assignee: ajvincent → nobody
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.)
Blocks: 558370
Blocks: 558651
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Depends on: 345624
OS: Windows XP → All
Hardware: x86 → All
Keywords: html5
Whiteboard: [parity-opera]
Blocks: 559309
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.
Attached patch Tests v1 (obsolete) — Splinter Review
Attachment #439737 - Flags: review?(jonas)
Attached patch Patch v1 (obsolete) — Splinter Review
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)
Attached patch Tests v1.1 (obsolete) — Splinter Review
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)
Attached patch Patch v1.1 (obsolete) — Splinter Review
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)
Depends on: 34582
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?
(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 :)
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.
Blocks: 561586
Blocks: 561620
Attached patch Tests v1.2 (obsolete) — Splinter Review
Minor changes in the tests: I've added a check for the invalid event.
Attachment #440191 - Attachment is obsolete: true
Attached patch Patch v1.2 (obsolete) — Splinter Review
r=sicking

Requested changes have been done.
Attachment #440192 - Attachment is obsolete: true
Attachment #441349 - Flags: superreview?(Olli.Pettay)
Attachment #441349 - Flags: superreview?(Olli.Pettay) → superreview+
Blocks: 562625
Blocks: 566348
Blocks: 558788
Blocks: 561635
Attached patch Patch v1.3 (obsolete) — Splinter Review
Updated to apply against current tip.
Merged with test patch.
Attachment #441348 - Attachment is obsolete: true
Attachment #441349 - Attachment is obsolete: true
Blocks: 583610
Attached patch Patch v1.3Splinter Review
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
This is now in the tree.
rev: http://hg.mozilla.org/mozilla-central/rev/acc8ccecdc90
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b5
Blocks: 581596
You need to log in before you can comment on or make changes to this bug.