User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060121 Camino/1.0b2+ Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.1) Gecko/20060121 Camino/1.0b2+ The search field at the top left of the apple site renders with an unnecessary border. Reproducible: Always Steps to Reproduce: Load Apple Downlad site Actual Results: Search field loaded with border Expected Results: Search field should be borderless

I believe Philippe has (coincidentally) gotten these Apple search boxes mostly figured out, and that we have some margins on our inputs that are the source of the problem (at least mostly). Apple's also using their own <input type="search"> invention here.

This referes to this snippet in forms.css (line 536 in my copy): input, input[disabled] { padding: 0px 1px 0px 1px; font-size: 11px; margin: 1px 1px 1px 1px; background-color: -moz-field; } There is no harm in removing those margins. Most other browsers don't have them anyway (Firefox, Opera, IE Mac and Win). Safari however does have margins on those text-field inputs. Removing those margins won't fix all the problems on Apple's site (some pages are just badly coded [*]), but most of them certainly yes. And it will probably improve rendering on some sites where everything is tightly fit, based on how IE Win render things. It might make the alignment problems with other widgets (submit buttons) a bit more visible, see this (draft) test file, but that is probably for another bug. http://dev.l-c-n.com/camino/forms.php * see this thread on the forums http://forums.mozillazine.org/viewtopic.php?t=371312

*** Bug 327928 has been marked as a duplicate of this bug. ***

*** Bug 328661 has been marked as a duplicate of this bug. ***

*** Bug 334413 has been marked as a duplicate of this bug. ***

Would it be kosher/hallal to insert a ruleset in forms.css for Apple's custom input[type="search"] field, so that we fix just that atm? I know that smfr added the margins, borders, etc., to the inputs for a reason when he did so. Doing that would allow us to fix this high-visibility problem now while we continue to wrangle over the defaults for generic inputs....

(In reply to comment #6) > Would it be kosher/hallal to insert a ruleset in forms.css for Apple's custom > input[type="search"] field, so that we fix just that atm? I know that smfr > added the margins, borders, etc., to the inputs for a reason when he did so. > > Doing that would allow us to fix this high-visibility problem now while we > continue to wrangle over the defaults for generic inputs.... input[type="search"] {margin:0;} would fix most of the display issues on Apple's site, except the iTunes page (http://www.apple.com/itunes/). Would it be possible to ship Camino with a userContent.css file (that includes that rule) by default ? That would possibly be more acceptable to some. I tested Apple's site with Firefox with that rule in my userContent.css file, checking most pages with the DomInspector, and it would solve the problem. (Probably better add a comment as well)

Implements the rule suggested by philippe to fix this until Cocoa forms.css are finalized. (Also accounts for disabled varieties, if such beasts exist.)

Comment on attachment 221283 [details] [diff] [review] Quick fix: special-case the Apple input[type="search"] extension I tested this a while back, and I can't seem to find anywhere this causes problems. r=me.

Comment on attachment 221283 [details] [diff] [review] Quick fix: special-case the Apple input[type="search"] extension Simon, can you sr this? This is a (temporary?) fix for the high-visibility Apple search boxes, but it preserves the OS L-a-F/Safari-parity margins on the rest of the normal input fields.

Comment on attachment 221283 [details] [diff] [review] Quick fix: special-case the Apple input[type="search"] extension Asking for branch-1.8.1 and 1.8.0.5 approval for this Camino-only Cocoa forms patch, which fixes a high-visibility issue with Apple's proprietary input[type="search"] boxes.

mento, what are the prospects of this getting in Cm102 via the minibranch? It doesn't necessarily need to be relnoted, and the natives have been agitating about it recently....

We'll take this for Camino 1.0.2. It's Cocoa-only and therefore Camino-only on the 1.8.0 and 1.8 branches, so we should get it there too and not need to worry about it on our minibranches.

Comment on attachment 221283 [details] [diff] [review] Quick fix: special-case the Apple input[type="search"] extension approved for 1.8.0 branch, a=dveditz for drivers

Landed trunk, 1.8, and 1.8.0.

Did you consider perhaps asking for **module owner or peer** review?

(In reply to comment #16) > Did you consider perhaps asking for **module owner or peer** review? smfr has r/sr'd Cocoa-only forms.css changes many times before (and has made many himself) since I've been with Camino, so this didn't seem any different to me. I *did* request approval-branch-181 from you, but dveditz granted it when he granted a=1.8.0.5.

Well, aside from the obvious mistake that I don't think you meant to style a disabled *element* that's *inside* an input with type="search", there's the bigger question of why you should have to do this specifically. Inputs with unknown types are supposed to be compatible with inputs with type="text". If that's not the case for the Camino forms.css, then that's a bug that needs to be fixed. If it is the case for Camino's forms.css, then I'm highly skeptical that you should break it for one particular unknown type to fix a single Web page. If there's really a compatibility issue you're probably better off fixing it for all text inputs.

(In reply to comment #17) > I *did* request approval-branch-181 from you, but dveditz granted it when he > granted a=1.8.0.5. Sorry, I was away for much of May, and I'm still not caught up.

The Cocoa section of forms.css tries to match the appearance of native Mac widgets (that's why there are margins on inputs) and, by extension, Safari. That's been the goal of the Cocoa section ever since the rules first landed, and they've been tweaked periodically to better match. Safari uses a native widget for input[type="search"] (the curved edges/looking glass and the input are an NSSearchField instead of img+input+img that other browsers get), so it doesn't have the particular problem we have here with the margin breaking the "widget", and Apple apparently only tested their HTML for non-Safari browsers against browsers without margins on inputs. So this "pseudo-widget" broke the layout of numerous Apple.com webpages rather noticeably in Camino, and Camino users have complained about it frequently.

Comment on attachment 225675 [details] [diff] [review] fixes the typo dbaron pointed out dbaron, in the meantime can I get your r and/or sr and/or approval-to-land for this typo fix?

As far as I can tell, this is a fix essentially designed to fix a single Web site, without regard to how it affects other sites. It seems like someone should determine whether the compatibility vs. appearance tradeoff for the margin is worth it or not and decide globally, not do something that's really a specific hack for one site. The fact that Safari supports <input type="search"> as something (slightly) special isn't all that relevant, since we don't. If you want to actually support what Safari does there' that's another story, and another discussion. But as long as we don't support it I'm not happy about treating it specially. Part of the process of changing Gecko should be considering how the changes affect all current and future Web sites, not just one, so I'm requesting that this patch be backed out so it can be considered properly.

dbaron, how does this negatively affect other sites?

I'm not saying it necessarily does. I'm saying it's up to the author of the patch to demonstrate that it doesn't (i.e., that it has been tested appropriately) to the module owners' satisfaction. Showing no sign of having considered the issue, or considered what invariants we previously tried to maintain that this patch breaks (such as inputs with unknown types behaving like type="text"), doesn't satisfy me. If this is a known Safari extension, what other sites use it? Have you tested whether this makes them better or worse? Have you written test pages that use it (unstyled and less styled, particularly) and seen whether it makes them better or worse? Or are you insisting that it's my job to do that work? I thought patch authors should test their patches appropriately before requesting review. Appropriate testing of this patch requires more than looking at pages on www.apple.com. For a patch going in to the 1.8.0 branch, the standard of testing should be rather high. And I'd also note that on the trunk this very soon won't be Camino-only anymore.

Before I leave entirely, a few final comments: As far as I can tell, there is only one real-world site that unconditionally implements a <input type="search">, http://blog.nickshanks.com/blog/ (and it's rather unstyled). Many designers seem very reluctant to use an Apple extension that will make their pages fail to validate, and, it seems, most of those that do implement the feature do so conditionally, only for Safari. See http://www.bartelme.at/journal_detail.php?detail=236 for an example of the conditional activation, and especially the comments for a summary of what still seems to be the running thought about implementing <input type="search">. Without access to Google's markup search engine or spending a significant amount of time browsing in Safari, it's difficult to tell, certainly. In the case of the one known real-world site, this patch had no noticeable difference. Apple is the only other implementor, and it's also the only one that, in an effort to be all things to all browsers, serves additional HTML "attached" to the elment (which extras Safari neatly ignores; someone ask hyatt how that trick works) to fake the appearance in non-Safari browsers, and it's these extra images that assume that all browsers other than Safari have no margins on their input elements (and Safari neatly ignores these images). Compare the search box on http://www.apple.com/trailers/ with and without the patch and see why we needed to do *something* from the users' perspective; the patch seemed to be the simplest way of fixing the problem quickly, and the one least likely to break other things, until we could sit down and polish Cocoa widgets and deal with thorny issues like allowing proper author styling (bug 320486; authors seem to love styling widgets out of recognizability while users seem to hate it), etc. Finally, I'll attach a patch based on another suggestion from philippe, where all generic/unspecified and unknown inputs have no margins whatsoever; I've not even given this a cursory test with any of the form widget testcases. Ave et vale.

Screenshot, Safari vs Camino on http://www.apple.com/trailers mentioned in comment #28 Camino is on the right

screenshot from http://www.apple.com/downloads/macosx/ mentioned in the url field of this bug (Camino is on the left) Both this screenshot and the previous one (comment #30) show the page using an unpatched version of Camino

So David is right -- unknown types should render like type="text" -- this is very clear in the HTML spec. If we're talking about creating a different control for type="search", compatibly with other UAs, that's fine. But if all we're doing is rendering a text input, we shouldn't be making it different from other text inputs. If the problem is that Apple's web designers are making unwarranted assumptions about margins on text inputs, we should talk to them about that and get them to fix; a single style rule on their part should do the trick, and they're already doing browser-sniffing here. If the problem is that their assumptions _are_ warranted but Camino violates them, we should change the default text input margins in Camino to be like all other UAs except Safari. But we should _not_ be changing the rendering of some text inputs just because we kinda want to fix up a single web site, imo.

v.fixed on 1.8.0 branch with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.5) Gecko/20060706 Firefox/1.5.0.5 and Camino Version 2006061318 (1.0.2).

Verifying on trunk with current nightly.

The problem isn't specifically with any other Web site. It's with future extensibility of Web standards. A future specification may want to add new values of the type attribute to the input element. (Web Forms 2 does, for example.) And the upgrade path to get authors to use those new values may rely on the fact that in old browsers, the inputs are treated like type="text". Being broken on one site isn't reason to break that invariant. The brokenness on that one site is either a sign of bad browser sniffing on that one site (which we shouldn't work around by breaking valuable invariants) or a wider incompatibility in our code that should be fixed. Please back out this patch.

We do not add workarounds for individual web sites into our core .css files. What's worse is that this patch did not get a peer review, and on top of that it was claimed to be a cairo-only patch when asking for approval for branches. Though I should take some responsibility for not catching that this was a false claim for the 1.5.0.5 approval. This patch needs to be backed out ASAP, from all branches. Unfortunatly it is too late to respin 1.5.0.5 just for this, but we should back it out for 1.5.0.6

Opps, i didn't see the #ifdef around the declaration. However I still strongly think this patch should be backed out. We just don't work around single websites like this, that's a very slippery slope. I also doubt that the patch is actually correct given that it refers to a <disabled> element, which I've never heard of.

Not cairo-only, but cocoa-only. It sounds to me like the fix should apply to Cocoa <input> text fields in general.

(In reply to comment #32) > If the problem is that Apple's web designers are making unwarranted assumptions > about margins on text inputs, we should talk to them about that and get them to > fix; a single style rule on their part should do the trick, and they're already > doing browser-sniffing here. Should we kick this to TE then? > If the problem is that their assumptions _are_ > warranted but Camino violates them, we should change the default text input > margins in Camino to be like all other UAs except Safari. How do we differentiate between warranted and unwarranted assumptions? :) I have no problems kicking this over to TE (though I have little faith in Apple to do the Right Thing here) if that's the proper course of action, but it's still unclear to me whether text inputs should assume a margin at all. It appears the origin of the margins for INPUT in general was the checkin to the old platform-forms.css to fix bug 228499 (which was really intended to fix the appearance of several buttons, not text fields). It seems they've stayed the same since their initial tweak. If we decide that input type=text (and unknown types) should have margin:0, is the best course of action then to ensure an appropriate margin is set on all *specific* input elements where we *do* want it, and dispose of the margin property on the general input selector altogether? cl

(In reply to comment #39) > If we decide that input type=text (and unknown types) should have margin:0, is > the best course of action then to ensure an appropriate margin is set on all > *specific* input elements where we *do* want it, and dispose of the margin > property on the general input selector altogether? That would be the best solution. Very recently, I visited a site that use a similar technique as Apple's, but to align an image with an input[type="text"]. Just as Apple, they didn't zero out the margin on the input widget. Note that other form controls also have problems with (vertical) margins (the 4px bottom margin on input[type="submit] and friends as an example. One would expect the label of those to be aligned with the text - the way they are in 'Save As' dialogue boxes a the OS level, btw.). In comment #2, I had posted a link to a test file illustrating some of those. http://dev.l-c-n.com/camino/forms.php

changes: * set margins on generic input, input[disabled] to 0 (zero) * change vertical margins on (set margin-bottom to 0) input[type="reset"], input[type="button"], input[type="submit"], input[type="reset"]:active:hover, input[type="button"]:active:hover, input[type="submit"]:active:hover input[type="reset"][disabled]:active, input[type="reset"][disabled], input[type="button"][disabled]:active, input[type="button"][disabled], input[type="submit"][disabled]:active, input[type="submit"][disabled] input[type="file"] > input[type="button"] I'm not sure if the (vertical) margins on textarea and select widgets need to be changed (currently 1px). Radio buttons and checkboxes are not touched either. They do have their own set of problems.

(In reply to comment #41) > Created an attachment (id=229040) [edit] > forms.css with some margin changes Lets try to keep changes on topic, please. This bug shouldn't change anything but the margins on Cocoa text-style inputs (type="text"/missing/unknown), and this file changes all inputs, changes button margins, and removes all the Cocoa conditionalizing (!?). All this bug needs is a patch to apply the current input margins specifically to all the inputs that currently have them, minus text and generic (and a testcase that shows that there aren't any unwanted side effects to general form layout). If button margins need to be changed, they need to be changed in another bug.

(In reply to comment #42) > (In reply to comment #41) > > Created an attachment (id=229040) [edit] > > forms.css with some margin changes > > Lets try to keep changes on topic, please. This bug shouldn't change anything > but the margins on Cocoa text-style inputs (type="text"/missing/unknown), and > this file changes all inputs, changes button margins, and removes all the Cocoa > conditionalizing (!?). > > All this bug needs is a patch to apply the current input margins specifically > to all the inputs that currently have them, minus text and generic (and a > testcase that shows that there aren't any unwanted side effects to general form > layout). If button margins need to be changed, they need to be changed in > another bug. In comment 2 I pointed out that setting the margins to 0 for generic input and input[type="text"] (*) would make the alignment problem more visible. Hence I added those changes for buttons (submit, reset, button) in my suggestion. But no problems, I opened bug 344483 for that problem of vertical alignment. (*) would apply to input [type="password"], input[type="file"] as well.

> How do we differentiate between warranted and unwarranted assumptions? :) They're warranted if all other websites make them too, basically -- in which case it's a de-facto standard that we should try to support unless there are good reasons not to. > is the best course of action then to ensure an appropriate margin is set on > all *specific* input elements where we *do* want it, and dispose of the > margin property on the general input selector altogether? I would say "yes".

Jonas: reassigning to you to explain what you think should happen now.

This backs out the patch that was landed. Whatever the fix for this bug is this isn't it. Especially given that firefox is switching over to cocoa widgets soon and so the original patch would no longer be camino-only.

Comment on attachment 233032 [details] [diff] [review] Backout patch Back out errorous forms.css change. Should be safe since this is just a backout of an earlier patch.

Checked in the backout patch

Comment on attachment 233032 [details] [diff] [review] Backout patch a=drivers, baby gets back!

Comment on attachment 233032 [details] [diff] [review] Backout patch approved for 1.8.0 branch, a=dveditz for drivers

Maybe the backout should have been in a new bug... does the fixed1.8.1 keyword mean you have or have not backed this out from 1.8.1?

The backout patch has now been checked in on the 1.8.1 and the 1.8.0 branches. Yeah, I'm not really sure how to flag this one. Especially since the original problem is still present (as the net checkin everywhere is now nothing). Feel free to reopen this bug if a correct fix is wanted.