Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support 'word-spacing' for geckolib #12795

Merged
merged 1 commit into from Aug 12, 2016
Merged

Conversation

@wafflespeanut
Copy link
Member

wafflespeanut commented Aug 10, 2016

These changes haven't been tested yet!


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes

This change is Reviewable

@highfive
Copy link

highfive commented Aug 10, 2016

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/gecko.mako.rs
@highfive
Copy link

highfive commented Aug 10, 2016

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!
@wafflespeanut
Copy link
Member Author

wafflespeanut commented Aug 10, 2016

@highfive highfive assigned Manishearth and unassigned glennw Aug 10, 2016
Percentage(f) => self.gecko.mWordSpacing.set_value(CoordDataValue::Percent(f)),
Calc(l_p) => self.gecko.mWordSpacing.set_value(CoordDataValue::Calc(l_p.into())),
},
None => self.gecko.mWordSpacing.set_value(CoordDataValue::Auto),

This comment has been minimized.

Copy link
@Manishearth

Manishearth Aug 10, 2016

Member

None should be zero I think. Or None. The spec says 0 but we might be able to get away with using None if gecko layout treats it as the same.

This comment has been minimized.

Copy link
@Manishearth

Manishearth Aug 10, 2016

Member

Yes, treat this the same as Length(0)


match v.0 {
Some(lop) => match lop {
Length(au) => self.gecko.mWordSpacing.set_value(CoordDataValue::Integer(au.0)),

This comment has been minimized.

Copy link
@Manishearth

Manishearth Aug 10, 2016

Member

I think this should be CoordDataValue::Coord

This comment has been minimized.

Copy link
@Manishearth

Manishearth Aug 10, 2016

Member

Yes, it should be coord.

@wafflespeanut wafflespeanut force-pushed the wafflespeanut:word_spacing branch from 9718dcb to 69c422f Aug 10, 2016
@wafflespeanut
Copy link
Member Author

wafflespeanut commented Aug 10, 2016

Comments make sense. I've made the changes :)

@Manishearth
Copy link
Member

Manishearth commented Aug 11, 2016

@bors-servo r+

This works.

@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

📌 Commit 69c422f has been approved by Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

Testing commit 69c422f with merge c4aae63...

bors-servo added a commit that referenced this pull request Aug 11, 2016
Support 'word-spacing' for geckolib

**These changes haven't been tested yet!**

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

- [ ] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12795)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

💔 Test failed - windows-dev

@Manishearth
Copy link
Member

Manishearth commented Aug 11, 2016

@bors-servo retry

  • github doesn't like bors
bors-servo added a commit that referenced this pull request Aug 11, 2016
Support 'word-spacing' for geckolib

**These changes haven't been tested yet!**

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

- [ ] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12795)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

Testing commit 69c422f with merge 45abdd6...

@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

Testing commit 69c422f with merge 26e3cf2...

bors-servo added a commit that referenced this pull request Aug 11, 2016
Support 'word-spacing' for geckolib

**These changes haven't been tested yet!**

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

- [ ] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12795)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

💔 Test failed - linux-rel

@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

📌 Commit 69c422f has been approved by Manishearth

@Manishearth
Copy link
Member

Manishearth commented Aug 11, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

Previous build results for arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, windows-dev are reusable. Rebuilding only linux-rel, mac-rel-wpt...

@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

💔 Test failed - mac-rel-wpt

@highfive
Copy link

highfive commented Aug 11, 2016

  ▶ TIMEOUT [expected PASS] /_mozilla/css/iframe/hide_layers2.html
@Manishearth
Copy link
Member

Manishearth commented Aug 11, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

Previous build results for arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, windows-dev are reusable. Rebuilding only mac-rel-wpt...

@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

Previous build results for arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, windows-dev are reusable. Rebuilding only mac-rel-wpt...

@bors-servo
Copy link
Contributor

bors-servo commented Aug 11, 2016

💔 Test failed - mac-rel-wpt

@highfive
Copy link

highfive commented Aug 11, 2016

  ▶ TIMEOUT [expected PASS] /_mozilla/css/iframe/hide_layers2.html
@Manishearth
Copy link
Member

Manishearth commented Aug 12, 2016

bors-servo added a commit that referenced this pull request Aug 12, 2016
Support 'word-spacing' for geckolib

**These changes haven't been tested yet!**

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

- [ ] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12795)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 12, 2016

Testing commit 69c422f with merge a229135...

@bors-servo
Copy link
Contributor

bors-servo commented Aug 12, 2016

@bors-servo bors-servo merged commit 69c422f into servo:master Aug 12, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@wafflespeanut wafflespeanut deleted the wafflespeanut:word_spacing branch Aug 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.