Closed Bug 1240181 Opened 4 years ago Closed 4 years ago

There's a red outline around URL bar in fx-team build on localhost URLs

Categories

(Firefox :: Theme, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 46
Tracking Status
firefox46 --- fixed

People

(Reporter: bgrins, Assigned: Gijs)

References

Details

(Keywords: regression)

Attachments

(1 file)

Attached image red-outline.png
I'm assuming this is due to Bug 1239319.  There's a CSS rule applying to the url input element:

:-moz-ui-invalid:not(output) {
    box-shadow: 0 0 1.5px 1px red;
}
I think this is because we trim the scheme for HTTP.

I think @inputmode="url"[1] should give us the OSK behaviour we want but without the styling (IIRC). I don't know how our support of inputmode on desktop is though.

[1] https://html.spec.whatwg.org/#attr-fe-inputmode
All bug 1239319 did was change the urlbar's nest input to use type="url" instead of type="autocomplete". :-moz-ui-invalid probably should only be applied to content anyways.
(In reply to Matthew N. [:MattN] from comment #1)
> I think this is because we trim the scheme for HTTP.

That makes sense.

> I think @inputmode="url"[1] should give us the OSK behaviour we want but
> without the styling (IIRC). I don't know how our support of inputmode on
> desktop is though.

Non-existent:

https://dxr.mozilla.org/mozilla-central/rev/02398f2be72b9bbf5ee79348b73ef122c915aae0/widget/windows/TSFTextStore.cpp#3023-3024

gets its value through:

https://dxr.mozilla.org/mozilla-central/rev/02398f2be72b9bbf5ee79348b73ef122c915aae0/dom/events/IMEStateManager.cpp#1014-1035

then there's separate support for inputmode, but that ends up in a different property which the TSF code doesn't consult.

We can hack the TSF code or we can just override the styling, presumably? Does the 'invalid' state actually have interaction consequences? I'm assuming it doesn't because there's no actual form and the input handlers and so on don't care at all...
(also, dom.forms.inputmode is false on release builds, https://dxr.mozilla.org/mozilla-central/rev/02398f2be72b9bbf5ee79348b73ef122c915aae0/modules/libpref/init/all.js#4806-4810 )

I think it'd be safer to override the styling here specifically than to try to make this not apply to chrome content. I could imagine it being useful for chrome HTML pages at some point.
(In reply to :Gijs Kruitbosch from comment #4)
>  I could imagine it being useful for chrome HTML pages at some point.

I believe we already rely on the styling in the editable password field of the remember password doorhanger so we shouldn't make this content-only.
It seems this *will* have behavioural consequences:

https://dxr.mozilla.org/mozilla-central/rev/02398f2be72b9bbf5ee79348b73ef122c915aae0/dom/html/HTMLInputElement.cpp#4376

It's hard to know offhand if any of those are going to be problematic.

On the one hand, this is making "just change TSFTextStore to specialcase the urlbar" seem like a quick hack that will avoid difficulties, and on the other, maybe this is an opportunity to actually make core DOM code do some of what we now have to do manually in the URL bar binding?
In my experience <input type="url"> for a field like our awesomebar which doesn't only accept URLs (consider searching for multiple terms with spaces) is problematic as some OSK won't even display a space bar.
i.e. semantically type="url" isn't really appropriate[1].

[1] https://html.spec.whatwg.org/#url-state-%28type=url%29
(In reply to Matthew N. [:MattN] from comment #8)
> i.e. semantically type="url" isn't really appropriate[1].
> 
> [1] https://html.spec.whatwg.org/#url-state-%28type=url%29

We only support desktop OSK on Windows, and there you do get a space bar. Other browsers do the same thing, so this is essentially parity with those browsers. There are actual user complaints on input about how there's no slash or .com key on the keyboard we present for the URL bar, and that makes user touch input much less efficient for URLs.
You're not replying to my point though. I agree we should fix the keyboard for users if Windows does include a space bar, what I'm saying is that our awesomebar doesn't meet the definition for type="url" and so another implementation should be used:

> The input element represents a control for editing a single absolute URL given in the element's value.
Backed out the urlbar part of bug 1239319:

remote:   https://hg.mozilla.org/integration/fx-team/rev/e0f242f0a68b
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
(In reply to Matthew N. [:MattN] from comment #10)
> You're not replying to my point though. I agree we should fix the keyboard
> for users if Windows does include a space bar, what I'm saying is that our
> awesomebar doesn't meet the definition for type="url" and so another
> implementation should be used:
> 
> > The input element represents a control for editing a single absolute URL given in the element's value.

Any value Firefox sets to it, if it changes, will be an absolute URL. The spec specifically allows any user input, if the field is editable, to not be an absolute URL, so in that sense I disagree that this violates the spec:

> User agents may allow the user to set the value to a string that is not 
> valid absolute URL, but may also or instead automatically escape characters
> entered by the user so that the value is always a valid absolute URL (even
> if that isn't the actual value seen and edited by the user in the interface)

Note "may", so it is not a requirement that the value is always a valid URL. In this case we just don't care about the "validity".

We can either file a new bug, use this bug, or set leave-open / reopen bug 1239319 to figure out what we do here.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
(In reply to :Gijs Kruitbosch from comment #14)
> (In reply to Matthew N. [:MattN] from comment #10)
> > You're not replying to my point though. I agree we should fix the keyboard
> > for users if Windows does include a space bar, what I'm saying is that our
> > awesomebar doesn't meet the definition for type="url" and so another
> > implementation should be used:
> > 
> > > The input element represents a control for editing a single absolute URL given in the element's value.
> 
> Any value Firefox sets to it, if it changes, will be an absolute URL. The
> spec specifically allows any user input, if the field is editable, to not be
> an absolute URL, so in that sense I disagree that this violates the spec:

Users edit search terms in the control which aren't a "single absolute URL".

> We can either file a new bug, use this bug, or set leave-open / reopen bug
> 1239319 to figure out what we do here.

I filed bug 1240208 to find a solution which doesn't lead to the red outline.
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.