search

gngrOrg / gngr

a cross-platform browser focussed on privacy.
https://gngr.info
283 stars 60 forks source link

border-*-width doesn't have default value #14

Open hrj opened 9 years ago

hrj commented 9 years ago

For the following style,

div {
  border-bottom-style: solid;
  border-bottom-color: green;
}

A border-bottom-width of 1px should be assumed.

Currently, it becomes 0px.

Need to check where the default needs to be decided (css parser, render-state or layout)

hrj commented 8 years ago

Test case here

jStyleParser is not inferring the border-*-width property. Will report upstream.

hrj commented 8 years ago

Added a comment to https://github.com/radkovo/jStyleParser/issues/16

hrj commented 8 years ago

There is no response from upstream yet. We can implement this in gngr itself.

@bogas04 Do you want to take a shot at it?

It requires changes in the class HtmlValues. It has a method called getBorderInfo which needs to automatically infer missing values for border-width, when other border values are specified.

hrj commented 8 years ago

On second thoughts, from a design perspective, it might be cleaner to do the inference in the caller of getBorderInfo which is StyleSheetRenderState.

bogas04 commented 8 years ago

Sure! Thanks for the location, will work on it tonight.

On 21-Oct-2015, at 11:20 AM, hrj notifications@github.com wrote:

There is no response from upstream yet. We can implement this in gngr itself.

@bogas04 Do you want to take a shot at it?

It requires changes in the class HtmlValues. It has a method called getBorderInfo which needs to automatically infer missing values for border-width, when other border values are specified.

— Reply to this email directly or view it on GitHub.

bogas04 commented 8 years ago

I was looking where the default border width was defined, and I came across this commented part of the code. Do you think it was intended to serve the same purpose ?

hrj commented 8 years ago

@bogas04 Yes, that might be relevant code. To be extra sure, you can try running the tests from grinder. I am going offline now but will be back later in the night, if you face any problems.

hrj commented 8 years ago

I fixed this in https://github.com/UprootLabs/gngr/commit/f1db56b356447a3b8d0c2f14b2a35698d46ca358

Didn't wait because it was affecting a lot of tests (44).

bogas04 commented 8 years ago

Sorry for being inactive, final sem exams upcoming and the old code wasn't keeping me interested enough. Still new to much of the codebase of gngr, much to explore and learn!

hrj commented 8 years ago

@bogas04 No worries...

and the old code wasn't keeping me interested enough.

LOL !

hrj commented 6 years ago

I am reopening this, because upstream replied after our fix and perhaps a different solution can be implemented.