Implement <input type="url">

RESOLVED FIXED in mozilla2.0b5

Status

()

defect
RESOLVED FIXED
13 years ago
9 years ago

People

(Reporter: WeirdAl, Assigned: mounir)

Tracking

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

Trunk
mozilla2.0b5
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [parity-opera], )

Attachments

(1 attachment, 7 obsolete attachments)

Reporter

Description

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

Updated

13 years ago
Summary: Implement <input type="uri"> → Implement <input type="url">
Reporter

Comment 1

13 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

13 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.

Updated

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

Updated

10 years ago
Assignee: ajvincent → nobody

Comment 7

9 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

9 years ago
Blocks: 558370
Assignee

Updated

9 years ago
Blocks: 558651
Assignee

Updated

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

Updated

9 years ago
Keywords: html5
Whiteboard: [parity-opera]
Assignee

Updated

9 years ago
Blocks: 559309
Assignee

Comment 8

9 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.
Assignee

Comment 10

9 years ago
Posted patch Tests v1 (obsolete) — Splinter Review
Attachment #439737 - Flags: review?(jonas)
Assignee

Comment 11

9 years ago
Posted 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)
Assignee

Comment 12

9 years ago
Posted 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)
Assignee

Comment 13

9 years ago
Posted 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)
Assignee

Updated

9 years ago
Depends on: 34582
Assignee

Updated

9 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

9 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 :)
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

9 years ago
Blocks: 561586
Assignee

Updated

9 years ago
Blocks: 561620
Assignee

Comment 20

9 years ago
Posted patch Tests v1.2 (obsolete) — Splinter Review
Minor changes in the tests: I've added a check for the invalid event.
Assignee

Updated

9 years ago
Attachment #440191 - Attachment is obsolete: true
Assignee

Comment 21

9 years ago
Posted 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+
Assignee

Updated

9 years ago
Blocks: 562625

Updated

9 years ago
Blocks: 566348
Assignee

Updated

9 years ago
Blocks: 558788
Assignee

Updated

9 years ago
Blocks: 561635
Assignee

Comment 22

9 years ago
Posted 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
Assignee

Updated

9 years ago
Blocks: 583610
Assignee

Comment 23

9 years ago
Posted 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
Assignee

Comment 24

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

Updated

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