Closed Bug 1634911 Opened 10 months ago Closed 9 months ago

Simplify loops over lines in block layout to use ranged for.

Categories

(Core :: Layout: Block and Inline, task, P5)

Product:
Core
Component:
Layout: Block and Inline
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla78
Tracking Flags:
Tracking Status
firefox78 --- fixed

People

(Reporter: emilio, Assigned: shishirlearnz, Mentored)

Details

Attachments

(1 file)

Bug 1634911 - Simplify loops over lines in block layout to use ranged for. r=emilio
47 bytes, text/x-phabricator-request
Details | Review

Loops like this would read so much nicer if they used ranged for.

Mentor: emilio

Yeah, there are many static helper functions in nsBlockFrame.cpp, so we might need to define a helper methods in nsBlockFrame like

nsLineList Lines() { return mLines; }

Then, we can forward iterate lines via

for (nsLineBox& line : Lines()) {
  ...
}

Or backward:

for (nsLineBox& line : Reversed(Lines())) {
  ...
}

Hi Ting-Yu,

I agree with what you. I'd discussed the same with emilio and am currently working over the patch. Please assign this bug to me.

Thanks,
Shishir

Assignee: nobody → shishirlearnz

Hi Emilio,

There were a few suggestions given by reviewbot after your approval of the patch. I'd addressed those comments and submitted the updated patch. Could you approve those changes as well and land the patch?

Many thanks. Have a nice day.

Regards,
Shishir

Flags: needinfo?(emilio)

Thanks, this looks great!

Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/39dff7adfc30
Simplify loops over lines in block layout to use ranged for. r=emilio

Re comment 4:

For a future reference, in Firefox's workflow, after a patch is approved it means there are only minor nits left to address, and the reviewer trusts the patch author to fix those nits. The author can land the patch whenever it's ready. (In Phabricator, use Add Action -> Change Project Tags -> add a "Check-in Needed" tag.) Someone or a bot will pick up your patch and land the patch.

I've fixed the error in the latest patch pushed via moz-phab.

Pushed by rmaries@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e708c9a6b38f
Simplify loops over lines in block layout to use ranged for. r=emilio

https://hg.mozilla.org/mozilla-central/rev/e708c9a6b38f

Status: NEW → RESOLVED
Closed: 9 months ago
status-firefox78: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Flags: needinfo?(shishirlearnz)
You need to log in before you can comment on or make changes to this bug.