Just Patch This! (or 101 for patching Firefox)

Contributing to Mozilla projects can be done in many ways. This week, a Webcompat bug about behavior differences in between browsers led me to propose a patch on Firefox and get it accepted. I learned a couple of things along the way. That might be useful for others.

The Webcompat Bug - Search Placeholder On Mobile Sites

It started with a difference of behavior in between Chrome and Firefox on Android devices for Nikkei Web site reported by kudodo from Mozilla Japan. I looked at the bug and checked the differences in Firefox 40 (Gecko 40) and Opera 30 (Blink/Chrome 43).

Indeed the placeholder text appears in Gecko and not in Blink.

<form id="searchBlank" action="" method="get">
    <p><input id="searchBox" placeholder="検索" name="" type="search"></p>
</form>

What was happening? I started the inspectors in Gecko and in Blink looking at the CSS properties used in both cases. The CSS didn't have any WebKit differences (for once).

On the Opera/Blink inspector, I noticed in the inherited properties from the user agent stylesheet (aka the default style that any Web page inherits from browsers).

Gecko didn't have the box-sizing: border-box; for input[type=search] in its default CSS.

input[type=search] {box-sizing: border-box;}

I tested in the inspector adding it to the stylesheet of the Web site, and noticed it was indeed fixing the issue in Gecko without creating an issue in Blink. Good! We need to contact the Web site so they can modify their CSS.

Gecko/Blink Default Rendering Differences

That said, fixing this particular Web site will not fix all the other sites on the Web which might exhibit the same issue. We also need to advocate for the same default rendering in Gecko (Firefox), Blink (Chrome, Opera, UCWeb, …), WebKit (Safari), Spartan (IE) and Edge (new MS browser). This led me to two questions:

1. What is the most common behavior across browsers?
2. Do Mozilla have a bug in bugzilla about this?

After testing, I found out that box-sizing: border-box; was the most common behavior for input[type=search] and that there was an open bug about input[type=search] rendering.

Patching Firefox User-Agent StyleSheet

I added a comment on the bug that it was creating a Web compatibility issue. Then I went on a journey to find where the user-agent stylesheet was in the source code repository of Mozilla (Quick Tip: You may want to search old bugs related to the same area in bugzilla, then you might find where you need to look at in the source code with previous patches). So I finally found that the user-agent stylesheet was managed in layout/style/ and that the file forms.css seemed to be pretty close from what I needed. I proposed a patch which was fixing the issue but was not perfect. I made mistakes and it's ok. Boris Zbarski and David Baron helped me to get it straight.

• The forms.css file had trailing white-spaces. My first patched fixed both the spaces issue and added the missing property. Boris invited my to make two separate patches.
• My first patch didn't contain a summary (aka commit message). I had done in the past patches with Mercurial for the UA override but I didn't know you could add a message at this stage. So David invited me to add proper messages on the patches.
• One another mistake I did was to use "Removing" instead of "Remove". David said: We usually write commit messages with verbs rather than nouns, so "Remove trailing spaces".

Creating A Patch On Mozilla Central

I'm assuming here that

These are the steps on the command line.

cd /your/path/mozilla-central
hg status
hg qpop -a
hg pull -u
cd layout/style
hg qnew bug1164783-spaces
# I'm using sublimetext. Choose a text editor.
subl forms.css
# checking if the diff makes sense
hg diff
# Adding a message to the patch
hg qrefresh -m "Bug 1164783 - Remove trailing spaces."
hg export qtip > ~/bug-1164783.patch
# creating another patch
hg qnew bug1164783-input
# doing the edits
hg diff
hg qrefresh -m "Bug 1164783 - Change default style for input type=search to box-sizing: border-box."
hg export qtip > ~/bug-1164783-input.patch

Once the modifications on the code are done and the patches are ready, you can attach the patch the bug on bugzilla.

• Browse File for choosing the patch you created
• select r? for the review and if you do not know who to ask for the review, there is usually a list of suggested reviewers.

If the the review is positive with r+ you can go on and add checkin-needed in the keywords of the bug. A committer will add your modifications to the next release of Firefox.

If the review is negative r-, read properly the comments which have been given in the review. Create another patch and attach it to the bug. At the same time, tick the box for the obsolete option on the previous patch.

The patch for input[search] should appear in Firefox 41.

Opening A Bug On HTML Specification

The input[type=search] default rendering was not defined on HTML specification. I opened a bug on W3C issue tracker so the specification could be fixed and if a group was implementing a new rendering engine, they would not have to reverse engineer the behavior of other browsers.

Otsukare!