Last Comment Bug 563884 - The link element should be display: none; in the UA style sheet
: The link element should be display: none; in the UA style sheet
Status: RESOLVED FIXED
: helpwanted, html5, student-project
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla24
Assigned To: Sebastian Wong
:
Mentors:
https://bugzilla.mozilla.org/attachme...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-05 01:42 PDT by Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-09-26)
Modified: 2013-05-22 09:38 PDT (History)
5 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Attempt at patch v1. (292 bytes, patch)
2013-05-20 13:11 PDT, Sebastian Wong
no flags Details | Diff | Splinter Review
reftest.list (40 bytes, text/plain)
2013-05-21 13:48 PDT, Sebastian Wong
no flags Details
html file with link script (397 bytes, text/plain)
2013-05-21 13:49 PDT, Sebastian Wong
no flags Details
html file without link script (135 bytes, text/plain)
2013-05-21 13:49 PDT, Sebastian Wong
no flags Details
563884 patch. (1.21 KB, patch)
2013-05-21 14:28 PDT, Sebastian Wong
no flags Details | Diff | Splinter Review
563884 patch (2.12 KB, patch)
2013-05-21 21:14 PDT, Sebastian Wong
bzbarsky: review+
Details | Diff | Splinter Review

Description Henri Sivonen (:hsivonen) (Not reading bugmail or doing reviews until 2016-09-26) 2010-05-05 01:42:30 PDT
Steps to reproduce:
 1) Load https://bugzilla.mozilla.org/attachment.cgi?id=392399

Actual results:
The link element participates in table layout.

Expected results:
Expected the link element not to participate in layout due to defaulting to display: none; per
http://www.whatwg.org/specs/web-apps/current-work/#the-css-user-agent-style-sheet-and-presentational-hints
Comment 1 Sebastian Wong 2013-05-19 13:28:24 PDT
Hi I'd love to work on this. Could you tell me where I should get started looking in the codebase to fix this?
Comment 2 Boris Zbarsky [:bz] (TPAC) 2013-05-19 18:40:35 PDT
Sebastian, see layout/style/html.css
Comment 3 Sebastian Wong 2013-05-20 13:11:32 PDT
Created attachment 751814 [details] [diff] [review]
Attempt at patch v1.

Added styling for the link tag. Change appears to fix the bug on my machine.
Comment 4 Boris Zbarsky [:bz] (TPAC) 2013-05-21 12:02:22 PDT
Sebastian, is that patch ready for review?  Do you want to add a reftest?  See https://developer.mozilla.org/en-US/docs/Creating_reftest-based_unit_tests
Comment 5 Sebastian Wong 2013-05-21 13:48:36 PDT
Created attachment 752356 [details]
reftest.list
Comment 6 Sebastian Wong 2013-05-21 13:49:02 PDT
Created attachment 752357 [details]
html file with link script
Comment 7 Sebastian Wong 2013-05-21 13:49:22 PDT
Created attachment 752358 [details]
html file without link script
Comment 8 Sebastian Wong 2013-05-21 13:51:41 PDT
I believe the patch is ready for review. I have not generated a patch with hg yet and the current attached patch is just a file diff. 

Without the proposed patch I get the following output when running the reftest: 

0:01.36 REFTEST TEST-START | file:///Users/sebastianwong/Documents/mozilla-central/bitmaptest/without_script.html | 0 / 1 (0%)
 0:01.47 REFTEST TEST-UNEXPECTED-FAIL | file:///Users/sebastianwong/Documents/mozilla-central/bitmaptest/with_script.html | image comparison (==), max difference: 255, number of differing pixels: 324

With the proposed patch I get the following output when running the reftest: 

0:01.28 REFTEST TEST-START | file:///Users/sebastianwong/Documents/mozilla-central/bitmaptest/without_script.html | 0 / 1 (0%)
 0:01.32 REFTEST TEST-PASS | file:///Users/sebastianwong/Documents/mozilla-central/bitmaptest/with_script.html | image comparison (==)
Comment 9 Boris Zbarsky [:bz] (TPAC) 2013-05-21 13:57:46 PDT
OK.  The patch looks good, but you do want to have the reftest in the patch.  Adding it to layout/reftests/bugs/reftest.list and putting the files in layout/reftests/bugs is probably fine.
Comment 10 Sebastian Wong 2013-05-21 14:28:08 PDT
Created attachment 752381 [details] [diff] [review]
563884 patch.

Added the changes you suggested. I'm still new so please let me know if theres anything I need to do
Comment 11 Boris Zbarsky [:bz] (TPAC) 2013-05-21 20:08:54 PDT
Comment on attachment 752381 [details] [diff] [review]
563884 patch.

It looks like you forgot to hg add the actual test files?
Comment 12 Sebastian Wong 2013-05-21 21:14:29 PDT
Created attachment 752521 [details] [diff] [review]
563884 patch

Whoops. There you go!
Comment 13 Boris Zbarsky [:bz] (TPAC) 2013-05-21 21:45:04 PDT
Comment on attachment 752521 [details] [diff] [review]
563884 patch

r=me if you put the new line in reftest.list in the right place in numerical order.

Thank you!
Comment 14 Ryan VanderMeulen [:RyanVM] 2013-05-22 05:01:31 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/74c44f375275
Comment 15 Ryan VanderMeulen [:RyanVM] 2013-05-22 09:38:32 PDT
https://hg.mozilla.org/mozilla-central/rev/74c44f375275

Note You need to log in before you can comment on or make changes to this bug.