Patches to Pale Moon

Talk about code development, features, specific bugs, enhancements, patches, and similar things.
Forum rules
Please keep everything here strictly on-topic.
This board is meant for Pale Moon source code development related subjects only like code snippets, patches, specific bugs, git, the repositories, etc.

This is not for tech support! Please do not post tech support questions in the "Development" board!
Please make sure not to use this board for support questions. Please post issues with specific websites, extensions, etc. in the relevant boards for those topics.

Please keep things on-topic as this forum will be used for reference for Pale Moon development. Expect topics that aren't relevant as such to be moved or deleted.
access2godzilla

Patches to Pale Moon

Unread post by access2godzilla » 2014-11-06, 07:01

Since PM still uses the Gecko/Firefox24 backend (with some improvements), I was thinking of applying a few patches to Pale Moon from BMO.

However, the changes would be too large to apply (along with the fact that not all patches are applicable). I would also suspect that not all patches would apply cleanly, since they might apply to a later version of Firefox, or, there are code changes in PM.

How should I go about doing this? My aim is to apply patches till PM's core can be equivalent to a current version of FF.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35647
Joined: 2011-08-28, 17:27
Location: Motala, SE

Re: Patches to Pale Moon

Unread post by Moonchild » 2014-11-06, 07:31

Thanks for wanting to help move Pale Moon forward!

"Not all patches would apply cleanly"... :)
You can expect that most patches don't apply cleanly; it depends a little on which part of the tree you are looking at patching, since not everything is touched by the "refactoring" madness, but most of the post-24 patches on BMO will have to be either ported manually or re-implemented from scratch.

It would also help, if you are planning to contribute the resulting changes to Pale Moon's source, to communicate first with me what you are planning to patch and which BMO bugpatches you are planning to apply. Not all "current FF core features" are desirable, so communicating first what you plan to do will prevent work that would be rejected for being unwanted, and the resulting disappointment and lost time&effort.

As to how to go about this in a practical sense? It's best if you clone the Pale Moon github repo, apply patches one bug/feature at a time, and create either a pull request or a patch file against the master branch (=trunk) after implementing and thoroughly testing (both in terms of build bustage on supported operating systems and the added/changed/related functionality) the changes made. If you re-implemented things or changed parts of code to satisfy dependency issues, please also list what you've done to solve the snags run into so I can evaluate the impact of those changes as well.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

access2godzilla

Re: Patches to Pale Moon

Unread post by access2godzilla » 2014-11-07, 01:57

I plan to patch the PM source with BMO patches; release-by-release of the Firefox source.

For now, I'm focusing on these features that are present in Firefox 25 <http://website-archive.mozilla.org/www. ... easenotes/>:
  • CSS3 background-attachment:local support to control background scrolling
  • iframe document content can now be specified inline
  • Many new ES6 functions implemented
  • Web Audio support
Can those be considered acceptable for Pale Moon?

New Tobin Paradigm

Re: Patches to Pale Moon

Unread post by New Tobin Paradigm » 2014-11-07, 02:29

access2godzilla wrote:I plan to patch the PM source with BMO patches; release-by-release of the Firefox source.

For now, I'm focusing on these features that are present in Firefox 25 <http://website-archive.mozilla.org/www. ... easenotes/>:
  • CSS3 background-attachment:local support to control background scrolling
  • iframe document content can now be specified inline
  • Many new ES6 functions implemented
  • Web Audio support
Can those be considered acceptable for Pale Moon?
We already have Web Audio Support and we already plan to implement inline content for iframes (which is a fairly good size job though /should/ be fairly straight forward - see bug 885289)

Regarding background-attachment: local it seems we already have it because it functions in Pale Moon exactly as it functions in latest trunk of Mozilla Australis

As far as I personally know we won't be enhancing JS in the way you suggest as JS is very temperamental and any change to it must be carefully evaluated.

access2godzilla

Re: Patches to Pale Moon

Unread post by access2godzilla » 2014-11-07, 10:42

Matt A Tobin wrote:We already have Web Audio Support
Is this a simple toggle of the pref, or code from later versions of the Mozilla codebase were integrated into Pale Moon?
Regarding background-attachment: local it seems we already have it because it functions in Pale Moon exactly as it functions in latest trunk of Mozilla Australis
Is this a custom implementation? It does not seem that the relevant BMO patches were applied to Pale Moon.
As far as I personally know we won't be enhancing JS in the way you suggest as JS is very temperamental and any change to it must be carefully evaluated.
Maybe you or Moonchild can personally look into this, since you suggest that JS related code must be carefully inspected?`

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35647
Joined: 2011-08-28, 17:27
Location: Motala, SE

Re: Patches to Pale Moon

Unread post by Moonchild » 2014-11-07, 11:07

I haven't started yet on iframes with inline content, so if you want to pick that up, that would help :)

It's fairly straightforward, and should be very easy to implement based on existing patches because of the fairly static nature of that subtree and close proximity of the patches to our base code. Bugs to note are bug #802895 for the core implementation and the followup that Tobin mentioned bug #885289 to restore desired context menu entries on such iframes. Please do check for any dependent other bugs that aren't necessarily mentioned by checking hg file revision history, but stay away from "refactoring" bugs.

Web Audio API:
@Tobin: we have a partial Web Audio API implementation, there are a few more features I never got around to, but I don't know if FF 25 specifically addressed any of those or just flipped the switch after more soak time.
@a2g: if you could investigate that, that would be great. Let me know what exactly the changes would be (relevant bug numbers) before you start patching though so I can have a look?

CSS background-attachment:local was added in 24.3.2 already. I'm not sure how you gather that patches were not applied since it was done very much in line with the BMO patches and you should be able to see relevant code in the same code snippets... Are you just trying to apply patches without looking at the code?

As for JS ES6 additions... something to treat extremely carefully. I don't want to inadvertently end up with broken or insecure JS. I've got an idea of how to handle this differently but that would be for a next milestone. If patches can be applied 100% cleanly, they are acceptable. If not, then they need careful consideration. You'll probably have to somehow figure out the exact order in which ES6 bugs were addressed by MozCo; do this last.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

access2godzilla

Re: Patches to Pale Moon

Unread post by access2godzilla » 2014-11-08, 02:33

Moonchild wrote:Let me know what exactly the changes would be (relevant bug numbers) before you start patching though so I can have a look?
879111,883591,886653,886657,886660,886690,888271,889016,889042,890023,890072,890248,891254,891543,891986,892492,894234,895720,900341
Moonchild wrote:I'm not sure how you gather that patches were not applied since it was done very much in line with the BMO patches
483446 and 877690 do apply to the code, hence the assumption.
Moonchild wrote:If patches can be applied 100% cleanly, they are acceptable.
886949 and 886847 apply cleanly, 866849 does not.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35647
Joined: 2011-08-28, 17:27
Location: Motala, SE

Re: Patches to Pale Moon

Unread post by Moonchild » 2014-11-08, 08:52

Please use the [bug] forum tag if you list bugzilla bugs in the future. it makes things a hell of a lot easier to check ;)
access2godzilla wrote:483446 and 877690 do apply to the code, hence the assumption.
No, they don't apply to the current code.

bug #483446 has already been implemented, as said in 24.3.2. Of course "applicable" to the code but already implemented and if trying to apply the patch you'd get some nice errors thrown by "patch" ;). Understand that I don't always pick up all reftest or other test code since we don't do automated (ref)tests - manual functionality testing is a much better approach for one-time implementations. So if you think the patch still applies because some parts of it apply, then you are mistaken.

bug #877690 is a big fat N/A because it is an internal API for autocomplete in devtools, completely unnecessary for Pale Moon even if you were to use the devtools, and not part of any spec. This is a good hint for other "applicable" BMO bugs you may find; We don't and never plan to implement things like telemetry and the likes, so any bugs related to that should never be taken.

ES6
access2godzilla wrote:886949 and 886847 apply cleanly, 866849 does not.
For the one that doesn't apply cleanly, please find out why.

WebAudio
That long string of bugs means you've got your work cut out for you, and you'd better organize this properly if you want to tackle that feature. Please create a feature dev branch in a cloned repo and create a pull request once you've implemented these and thoroughly tested them, so I can easily manage the bugs in the future (allowing me to check blame etc.). Please be aware that later sec fixes have been applied to the source in that area and if something doesn't apply cleanly because the code is actually different, it may be because of that - and overwriting the other code is not a good thing in that case because you'd undo the sec patch.

I suggest if you want to start this patching run, start with the inline content iframes, but in the end it's your choice.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

access2godzilla

Re: Patches to Pale Moon

Unread post by access2godzilla » 2014-11-09, 03:46

Inline iframe content:
These did not apply cleanly. I'm not really sure how to do that manually:
https://hg.mozilla.org/mozilla-central/rev/da373b72ad88
https://hg.mozilla.org/mozilla-central/rev/693d37696683

ES6
It's just the 1st part (that involves a test) that does not apply. You may also have to look into https://bugzilla.mozilla.org/show_bug.cgi?id=904723

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35647
Joined: 2011-08-28, 17:27
Location: Motala, SE

Re: Patches to Pale Moon

Unread post by Moonchild » 2014-11-09, 09:09

I'm sorry but You might want to reconsider your offer if you "don't know how to apply patches manually" and recode/adapt patch code to our source. Because that is what you would have to do 95% of the time.

Just forwarding me to the relevant bugs and commits really doesn't help. I know where to find that stuff, y'know? ;-)
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

New Tobin Paradigm

Re: Patches to Pale Moon

Unread post by New Tobin Paradigm » 2014-11-09, 22:26

I am sorry a2g but I really wanted inline iframe content in 25.1 so I went ahead and ported it to the codebase. Please do continue your exploration and learning as it is appreciated. Looking forward to see what you have in mind next :)

access2godzilla

Re: Patches to Pale Moon

Unread post by access2godzilla » 2014-11-21, 12:23

I recently have not had the time these days to look into this any further, but atleast consider applying 886949 and 886847. 866849 seems to have never landed in mozilla-central.

New Tobin Paradigm

Re: Patches to Pale Moon

Unread post by New Tobin Paradigm » 2014-11-21, 12:58

access2godzilla wrote:I recently have not had the time these days to look into this any further, but atleast consider applying 886949 and 886847. 866849 seems to have never landed in mozilla-central.
And those are?

access2godzilla

Re: Patches to Pale Moon

Unread post by access2godzilla » 2014-11-21, 13:08

They add some ES6 functions:
886949 - Add Number.parseInt and Number.parseFloat
866847 - Implement Map#forEach and Set#forEach

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35647
Joined: 2011-08-28, 17:27
Location: Motala, SE

Re: Patches to Pale Moon

Unread post by Moonchild » 2014-11-21, 15:33

I'm actually looking at doing much more extensive implementations than just those.
Also, it would help if you would actually use the "bug" bbcode tag that I've implemented on the forum when referring to bugs by number, to have an easy bugzilla link created ;)
bugtag.png
You do not have the required permissions to view the files attached to this post.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

access2godzilla

Re: Patches to Pale Moon

Unread post by access2godzilla » 2014-11-25, 17:29

You may also consider applying this: bug #896287, it fixes some reported crashes with Nvidia drivers.